Skip to content

Commit fbc21e6

Browse files
committed
refactor: tighten the comments on this change and the quota classification
Both sets explained the incident that motivated the code rather than the code itself. That kind of narrative stops being true as the surrounding system moves and starts misleading instead, so each is cut back to the reason a reader needs.
1 parent 8b8c9d9 commit fbc21e6

4 files changed

Lines changed: 17 additions & 39 deletions

File tree

apps/sim/lib/embeddings/client.test.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -823,11 +823,8 @@ describe('knowledge embedding transport fallback', () => {
823823
})
824824

825825
/**
826-
* OpenAI returns 429 for an exhausted balance as well as for a rate limit, but
827-
* only one of them reopens. Retrying a spent account cannot succeed, and since
828-
* the sweep re-queues failed documents every sync it turns into permanent load
829-
* — this was observed burning every attempt on thousands of documents for
830-
* weeks against an account with no credit.
826+
* A spent account never reopens, and the sweep re-queues failed documents every
827+
* sync — so retrying one burns the budget per document, indefinitely.
831828
*/
832829
it('does not retry a 429 that reports an exhausted balance', async () => {
833830
vi.useFakeTimers()

apps/sim/lib/embeddings/client.ts

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,7 @@ const EMBEDDING_RETRY_BUDGET_MS = EMBEDDING_MAX_RETRIES * EMBEDDING_MAX_RETRY_DE
7979
export class EmbeddingAPIError extends Error {
8080
public status: number
8181

82-
/**
83-
* The provider rejected this for an exhausted balance rather than a rate that
84-
* will recover. Both arrive as 429.
85-
*/
82+
/** Rejected for an exhausted balance rather than a recoverable rate. Both are 429. */
8683
public quotaExhausted?: boolean
8784

8885
/**
@@ -100,13 +97,8 @@ export class EmbeddingAPIError extends Error {
10097

10198
/**
10299
* True when a rejection body reports an exhausted balance rather than a rate
103-
* limit.
104-
*
105-
* OpenAI returns 429 for both, but only one of them reopens. `insufficient_quota`
106-
* stands until somebody adds credit, so retrying it cannot succeed no matter how
107-
* long the loop waits — and because a failed document is re-queued by the sweep
108-
* on every sync, an account that has run out turns into a permanent load: the
109-
* budget is spent per document, per attempt, forever.
100+
* limit. OpenAI returns 429 for both, but only a rate limit reopens: a spent
101+
* account stands until someone adds credit, so retrying it cannot succeed.
110102
*/
111103
function isQuotaExhaustionBody(errorText: string): boolean {
112104
try {
@@ -148,13 +140,10 @@ function statedWaitOutlastsBudget(error: unknown): boolean {
148140
}
149141

150142
/**
151-
* Whether another attempt against the same provider could plausibly succeed.
152-
*
153-
* Deliberately narrower than {@link isTransientEmbeddingError}, which also
154-
* decides whether the fallback chain should try a *different* provider. Those
155-
* two questions differ: an exhausted balance rules out the key we just used, but
156-
* says nothing about the next one in the chain, so a quota rejection stops the
157-
* retries here while remaining eligible for failover.
143+
* Whether another attempt against the *same* provider could succeed. Narrower
144+
* than {@link isTransientEmbeddingError}, which decides whether to fail over to a
145+
* different one: an exhausted balance rules out the key just used but says
146+
* nothing about the next in the chain.
158147
*/
159148
function isWorthRetrying(error: unknown): boolean {
160149
if (!isTransientEmbeddingError(error)) return false

apps/sim/lib/knowledge/documents/service.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -756,11 +756,9 @@ async function dispatchViaBatchTrigger(
756756
}
757757

758758
/**
759-
* Only a total dispatch failure raises, so a chunk that failed on its own used
760-
* to leave its documents sitting at `pending` with nothing recording why —
761-
* invisible until the stuck-document sweep happened to pick them up, and never
762-
* if they aged out of its window first. Running them here costs the caller time
763-
* it hoped to hand to the queue, which is the point: the work still happens.
759+
* Only a total dispatch failure raises, so a chunk failing alone would leave its
760+
* documents at `pending` with nothing recording why. Processing them here is
761+
* slower than the queue but does not drop the work.
764762
*/
765763
if (undispatched.length > 0) {
766764
logger.warn(

apps/sim/trigger.config.ts

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,11 @@ export default defineConfig({
7676
syncEnvVars(() => [
7777
{ name: 'DB_APP_NAME', value: 'sim-trigger' },
7878
/**
79-
* Workers run Trigger.dev by definition, but the flag that says so is
80-
* read from the environment and was only ever set on the app container.
81-
* `isTriggerAvailable()` therefore returned false inside every worker, so
82-
* anything a task dispatched — document processing above all — silently
83-
* took the in-process path instead of the queue it was written for. A
84-
* connector sync ended up chunking and embedding thousands of documents
85-
* itself, five at a time, and running until it hit its max duration.
86-
*
87-
* Safe to assert here because the check also requires TRIGGER_SECRET_KEY,
88-
* which only the Trigger.dev runtime provides: where dispatching is not
89-
* actually possible this stays false and nothing changes.
79+
* Workers run Trigger.dev by definition, but the flag saying so was only
80+
* set on the app container, so `isTriggerAvailable()` was false in every
81+
* task run and dispatched work silently took the in-process fallback.
82+
* Ineffective where dispatching is impossible: the check also requires
83+
* TRIGGER_SECRET_KEY, which only the Trigger.dev runtime provides.
9084
*/
9185
{ name: 'TRIGGER_DEV_ENABLED', value: 'TRUE' },
9286
]),

0 commit comments

Comments
 (0)