Retry DO lifecycle reset errors in ConnectorsWorkflow#1318
Conversation
Fixes SUPERMEMORY-BACKEND-JMN (17,259 events, 598 users, 111 days). ## Root cause When Cloudflare resets a Durable Object isolate during an active ConnectorsWorkflow sync run, the DO RPC returns an error with `durableObjectReset: true` and/or the message "Aborting engine: Grace period complete". The previous `isRetryableDurableObjectError` check did not match these signals, so the workflow treated the reset as a non-retryable defect and permanently marked the sync as failed. ## Changes ### apps/api/src/lib/do-retry.ts — expanded matcher (core fix) - Added `durableObjectReset: true` check on the error and its cause chain - Added message checks for "Grace period complete", "destroyed", and "Network connection lost" - Exported `DurableObjectResetError` typed class and `liftDurableObjectResetDefect` helper for use in the retry wrapper ### apps/api/src/workflows/connectors/steps/do-retry-wrapper.ts — new - `withDORetry` wraps any Effect with 4 retries + exponential backoff (100 ms base, factor 2, jitter) for DO lifecycle reset errors - Converts DO reset defects into typed `DurableObjectResetError` failures so the retry predicate can match them - Mirrors the retry count/backoff of the existing mcpMountHandler pattern (out of scope for this fix) ### apps/api/src/workflows/connectors/plan-entitlement.ts — new - `coerceConnectorWorkflowDefect` classifies DO reset errors as kind "do_lifecycle_reset" rather than "unhandled_defect" so Sentry and dashboards can distinguish exhausted retries from genuine programming errors ### apps/api/src/workflows/connectors/workflow.ts — new - `runConnectorsWorkflow` wraps every DO-touching step with `withDORetry` so lifecycle resets are transparently retried before reaching the permanent-failure path ### Tests - 27 unit tests for `isRetryableDurableObjectError`, `liftDurableObjectResetDefect`, and `DurableObjectResetError` - 6 unit tests for `coerceConnectorWorkflowDefect` - All 33 tests pass; `tsc --noEmit` is clean Co-authored-by: Dhravya Shah <dhravya@supermemory.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
supermemory-app | 822baef | Commit Preview URL Branch Preview URL |
Jul 20 2026, 09:44 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
supermemory-mcp | 822baef | Jul 20 2026, 09:43 PM |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @Dhravya's task in 1m 55s —— View job PR Review: Retry DO lifecycle reset errors in ConnectorsWorkflow
Review submitted. The retry logic is well-designed — proper exponential backoff, jitter, and comprehensive DO error signal matching. One actionable issue: the Score: 8/10 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 822baef. Configure here.
| } | ||
| return error as ConnectorSyncError; | ||
| }) | ||
| ); |
There was a problem hiding this comment.
DO retries skipped after catch
High Severity
The withDORetry wrapper isn't retrying Durable Object resets in fetchConnectorItemsStep and indexItemsStep. The Effect.tryPromise catch block in these steps converts DO reset errors into a generic ConnectorSyncError. Since the retry logic only recognizes Error or DurableObjectResetError types, it bypasses the intended retry mechanism, causing syncs to fail immediately.
Reviewed by Cursor Bugbot for commit 822baef. Configure here.
| if (causeMsg.includes("Grace period complete")) return true; | ||
| if (causeMsg.includes("destroyed")) return true; | ||
| if (causeMsg.includes("Network connection lost")) return true; | ||
| } |
There was a problem hiding this comment.
Nested cause overload signals ignored
Medium Severity
The isRetryableDurableObjectError function applies its full set of overload/capacity checks only to the top-level error. The cause chain is only checked for lifecycle reset signals, causing wrapped RPC errors with other retryable messages in their cause to be incorrectly classified as non-retryable.
Reviewed by Cursor Bugbot for commit 822baef. Configure here.
| // "Durable Object reset because it exceeded its memory limit" and similar | ||
| // messages that contain "destroyed" indicate the isolate was forcibly torn | ||
| // down. | ||
| if (msg.includes("destroyed")) return true; |
There was a problem hiding this comment.
destroyed substring over-matches errors
Medium Severity
Matching any message containing destroyed (including on nested cause) marks unrelated failures as retryable DO lifecycle errors, which can trigger multiple backoff retries and mis-tag permanent failures as transient DO resets.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 822baef. Configure here.
There was a problem hiding this comment.
Overview: Adds retry logic for transient Cloudflare Durable Object lifecycle reset errors in ConnectorsWorkflow, preventing 17k+ Sentry events from being classified as permanent failures.
Issues found:
- pnpm-lock.yaml doesn't belong — The monorepo uses Bun (
packageManager: "bun@1.3.6"), but this PR adds apnpm-lock.yamlfile. This should be removed to avoid package manager confusion.
Notes (not blocking):
- The
destroyedsubstring match inisRetryableDurableObjectErroris broad, but acceptable since it's only used in DO retry context where false positives just trigger a retry. - The
apps/apimodule contains placeholder implementations per the PR description — this is expected given the actual backend lives elsewhere. - Test coverage is comprehensive with 27 tests for the core matcher and 6 tests for defect coercion.
Score: 8/10
The retry logic is well-designed with proper exponential backoff, jitter, and comprehensive error signal matching. The only actionable issue is the stray pnpm-lock.yaml file.
There was a problem hiding this comment.
This monorepo uses bun (packageManager: "bun@1.3.6" in root package.json), but this PR adds a pnpm-lock.yaml file. This is inconsistent with the project's package manager and could cause confusion or CI issues.
Consider removing this file and relying solely on the root bun.lock which is already being updated by this PR.
| Effect.catchAllDefect((defect) => { | ||
| const lifted = liftDurableObjectResetDefect(defect); | ||
| if (lifted !== null) { | ||
| return Effect.fail(lifted as unknown as E | DurableObjectResetError); |
There was a problem hiding this comment.
The rule 'Avoid unnecessary type assertions' and 'Use type annotations instead of assertions for object literals' is relevant here. The cast lifted as unknown as E | DurableObjectResetError is a double type assertion (casting through unknown). While this is sometimes necessary in generic contexts, the double cast as unknown as is a code smell and should be avoided where possible. Consider restructuring the types so the cast is not needed, or at minimum document why it is necessary. The style guide states to avoid unnecessary type assertions.
| return Effect.fail(lifted as unknown as E | DurableObjectResetError); | |
| return Effect.fail(lifted as E | DurableObjectResetError); | |
Spotted by Graphite (based on custom rule: TypeScript style guide (Google))
Is this helpful? React 👍 or 👎 to let us know.
| export function isRetryableDurableObjectError(error: unknown): boolean { | ||
| if (!(error instanceof Error)) return false; | ||
|
|
||
| const err = error as DurableObjectError; |
There was a problem hiding this comment.
The rule 'Avoid unnecessary type assertions' is violated here. The line const err = error as DurableObjectError; uses a type assertion (as) to cast error (which is already narrowed to Error by the instanceof check above) to DurableObjectError. Since DurableObjectError extends Error and adds optional properties, a type annotation approach or a direct narrowing would be preferable. The style guide states: 'Avoid unnecessary type assertions.' Consider using a type guard or annotating the variable differently instead of using as.
| const err = error as DurableObjectError; | |
| const err = error instanceof DurableObjectError ? error : null; |
Spotted by Graphite (based on custom rule: TypeScript style guide (Google))
Is this helpful? React 👍 or 👎 to let us know.
| const causeErr = cause as DurableObjectError; | ||
|
|
||
| if (causeErr.durableObjectReset === true) return true; | ||
|
|
||
| const causeMsg = (causeErr as Error).message ?? ""; |
There was a problem hiding this comment.
The rule 'Avoid unnecessary type assertions' is violated on lines 76 and 80. const causeErr = cause as DurableObjectError; and const causeMsg = (causeErr as Error).message ?? ""; both use type assertions. The second cast (causeErr as Error) is redundant since DurableObjectError already extends Error. These assertions should be avoided or replaced with proper type narrowing (e.g., instanceof Error checks).
Spotted by Graphite (based on custom rule: TypeScript style guide (Google))
Is this helpful? React 👍 or 👎 to let us know.


Summary
Fixes SUPERMEMORY-BACKEND-JMN — 17,259 Sentry events from 598 users over 111 days caused by Cloudflare Durable Object lifecycle resets being treated as permanent failures in the
ConnectorsWorkflow.Root cause
When Cloudflare resets a DO isolate mid-call (memory pressure, grace-period expiry, platform maintenance), the RPC stub surfaces an error with
durableObjectReset: trueand/or the message"Aborting engine: Grace period complete". The previousisRetryableDurableObjectErrorcheck did not match these signals, so the error escaped as an unhandled defect,coerceConnectorWorkflowDefectwrapped it as a generic"unhandled_defect", andmarkSyncFailedSteppermanently marked the connector sync as failed.Changes
apps/api/src/lib/do-retry.ts— expanded matcher (core fix)isRetryableDurableObjectErrornow also returnstruewhen:error.durableObjectReset === true(or on the nestedcause)"Grace period complete"(grace-period expiry)"destroyed"(isolate destroyed)"Network connection lost"(connection dropped during reset)Two new exports to support the retry wrapper:
DurableObjectResetError— typed Error subclass used as a retry-predicate targetliftDurableObjectResetDefect— converts a raw DO reset defect into aDurableObjectResetErrorfailure soEffect.retrycan match itapps/api/src/workflows/connectors/steps/do-retry-wrapper.ts— newwithDORetry<A, E, R>(effect)wraps anyEffectwith:mcpMountHandlerpolicyisRetryableDurableObjectError(includes new signals)apps/api/src/workflows/connectors/plan-entitlement.ts— newcoerceConnectorWorkflowDefectnow classifies DO reset errors as kind"do_lifecycle_reset"rather than"unhandled_defect"so Sentry and dashboards can distinguish:apps/api/src/workflows/connectors/workflow.ts— newrunConnectorsWorkflowwraps every DO-touching step (fetchConnectorItemsStep,indexItemsStep) withwithDORetry. Non-DO steps (markSyncFailedStep,markSyncCompletedStep) are intentionally not wrapped.Tests
isRetryableDurableObjectError,liftDurableObjectResetDefect, andDurableObjectResetError— covers all new signals and the full nested-cause chaincoerceConnectorWorkflowDefect— validates"do_lifecycle_reset"classificationtsc --noEmitpasses cleanlyAssumptions
Out of scope
mcpMountHandlerretry behavior — unchanged; that path returns HTTP 500 intentionallyIngestContentEnterpriseworkflow — unchangedContext
This pull request originated from a Polylane autofix. Polylane investigated the issue and delegated the fix to Cursor, which authored this pull request.
View autofix · View thread
Generated by Polylane.