fix(webapp): downgrade retryable directory-sync effect failures to warn#4200
fix(webapp): downgrade retryable directory-sync effect failures to warn#42000ski wants to merge 1 commit into
Conversation
Directory-sync effects are idempotent and the accounts-webhook worker retries the whole event, so a single failed attempt (typically a role assignment losing a serializable race during a backfill burst) is self-healing rather than alert-worthy. Tag those thrown errors with logLevel "warn" so the worker logs at warn instead of error, keeping them visible for triage without paging.
|
WalkthroughAdds a helper function, retryableEffectError, in directorySyncEffects.server.ts that creates an Error object tagged with logLevel: "warn". This helper replaces plain Error throws in two failure paths within the applyEffect switch: the "provision" role overwrite failure and the "set_role" failure, changing the logging severity of these thrown errors used in the worker retry flow. No exported or public entity signatures are changed. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // Directory-sync effects are idempotent and the accounts-webhook worker retries | ||
| // the whole event (maxAttempts: 5). A single failed attempt is therefore an | ||
| // expected, self-healing condition rather than an alert-worthy error — the most | ||
| // common case is a role assignment losing a serializable race during a backfill | ||
| // burst (the plugin already retries that internally; the worker retry mops up | ||
| // the rest). Tag the thrown error with `logLevel: "warn"` so the worker logs it | ||
| // at warn instead of error (see redis-worker `processItem`), keeping it visible | ||
| // for triage without paging as an error on every transient retry. | ||
| function retryableEffectError(message: string): Error { | ||
| return Object.assign(new Error(message), { logLevel: "warn" as const }); | ||
| } |
There was a problem hiding this comment.
🔍 DLQ escalation also respects the warn log level
When the redis-worker exhausts all retry attempts and moves the item to the dead-letter queue, it also checks errorLogLevel and logs at warn instead of error (packages/redis-worker/src/worker.ts:812-814 for batch items, packages/redis-worker/src/worker.ts:971-975 for single items). This means that even a permanently failing directory-sync effect (e.g., a role that was deleted between retries) will only log at warn when it hits the DLQ. If permanent failures should escalate to error-level alerting after exhausting retries, this would need the worker to distinguish transient vs. permanent, or the logLevel approach would need to be applied per-attempt rather than on the error object. Worth confirming this is the desired behavior for the DLQ case.
Was this helpful? React with 👍 or 👎 to provide feedback.
Directory-sync effects are idempotent and the accounts-webhook worker retries
the whole event, so a single failed attempt (typically a role assignment
losing a serializable race during a backfill burst) is self-healing rather
than alert-worthy. Tag those thrown errors with logLevel "warn" so the worker
logs at warn instead of error, keeping them visible for triage without paging.