Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions packages/worker/src/d1-retry.node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,20 @@ test('runD1WithRetry matches lock errors, retries them, and rethrows other failu
new Error('Currently processing a long-running export.'),
),
).toBe(true)
expect(
isRetryableD1LockError(new Error('D1_ERROR: Network connection lost.')),
).toBe(true)
expect(isRetryableD1LockError(new Error('Network connection lost.'))).toBe(
true,
)
expect(
isRetryableD1LockError(new Error('Error: Network connection lost.')),
).toBe(true)
expect(
isRetryableD1LockError(
new Error('Network connection lost while uploading...'),
),
).toBe(false)
expect(isRetryableD1LockError(new Error('syntax error near SELECT'))).toBe(
false,
)
Expand Down Expand Up @@ -62,6 +76,20 @@ test('runD1WithRetry matches lock errors, retries them, and rethrows other failu
vi.useRealTimers()
}

vi.useFakeTimers()
const networkRetryOperation = vi
.fn()
.mockRejectedValueOnce(new Error('D1_ERROR: Network connection lost.'))
.mockResolvedValueOnce('ok')
try {
const resultPromise = runD1WithRetry(networkRetryOperation)
await vi.advanceTimersByTimeAsync(d1LockRetryBaseDelayMs)
await expect(resultPromise).resolves.toBe('ok')
expect(networkRetryOperation).toHaveBeenCalledTimes(2)
} finally {
vi.useRealTimers()
}

const failingOperation = vi
.fn()
.mockRejectedValue(new Error('D1_ERROR: syntax error near INSERTZ'))
Expand Down
34 changes: 28 additions & 6 deletions packages/worker/src/d1-retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,32 @@ function sleep(ms: number) {
export const d1LongRunningExportMessage =
'Currently processing a long-running export'

/**
* Cloudflare D1 binding transport blip (`D1_ERROR: Network connection lost.`).
* Not an application defect — the Worker lost its session to D1 mid-query.
* Same retry / Sentry-drop class as SQLITE_BUSY and long-running exports.
* Match only the exact D1 forms (optional `Error:` / `D1_ERROR:` prefixes) so
* unrelated "Network connection lost while …" messages stay out.
*/
export const d1NetworkConnectionLostMessage = 'Network connection lost'

function isD1NetworkConnectionLostMessage(message: string) {
const normalized = message
.trim()
.replace(/^Error:\s*/i, '')
.replace(/^D1_ERROR:\s*/i, '')
return (
normalized === d1NetworkConnectionLostMessage ||
normalized === `${d1NetworkConnectionLostMessage}.`
)
}

export function isRetryableD1LockMessage(message: string) {
return (
message.includes('SQLITE_BUSY') ||
message.includes('database is locked') ||
message.includes(d1LongRunningExportMessage)
message.includes(d1LongRunningExportMessage) ||
isD1NetworkConnectionLostMessage(message)
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

Expand All @@ -42,12 +63,13 @@ export function isRetryableD1LockSentryEvent(event: ErrorEvent) {
}

/**
* Retries transient D1 unavailability: SQLITE_BUSY lock contention and
* Retries transient D1 unavailability: SQLITE_BUSY lock contention,
* Cloudflare's "Currently processing a long-running export" (DR / REST
* exports block other requests). D1 does not automatically retry write
* queries, so cron lanes and long-running retention batches need
* application-level backoff when they overlap with concurrent writers or
* an in-flight export.
* exports block other requests), and binding "Network connection lost"
* transport blips. D1 does not automatically retry write queries, so cron
* lanes and long-running retention batches need application-level backoff
* when they overlap with concurrent writers, an in-flight export, or a
* brief D1 session drop.
*/
export async function runD1WithRetry<T>(
operation: () => Promise<T>,
Expand Down
22 changes: 22 additions & 0 deletions packages/worker/src/sentry-options.node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,28 @@ test('filterSentryEvent drops expected platform and caller noise and keeps real
}),
).toBeNull()

expect(
filterSentryEvent({
exception: {
values: [{ value: 'Network connection lost.' }],
},
}),
).toBeNull()
expect(
filterSentryEvent({
exception: {
values: [{ value: 'D1_ERROR: Network connection lost.' }],
},
}),
).toBeNull()

const unrelatedNetworkLoss = {
exception: {
values: [{ value: 'Network connection lost while uploading...' }],
},
}
expect(filterSentryEvent(unrelatedNetworkLoss)).toBe(unrelatedNetworkLoss)

const userModuleBuildFailure = {
exception: {
values: [
Expand Down
7 changes: 4 additions & 3 deletions packages/worker/src/sentry-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,10 @@ export function buildSentryOptions(env: Env): CloudflareOptions {
// D1 marks SQLITE_BUSY with NOSENTRY at the storage layer, but
// application capture paths (for example scheduled_lane_failed) still
// forwarded them. The same applies to "Currently processing a
// long-running export" while the nightly DR D1 export holds the DB.
// These are transient platform unavailability errors retried in app
// code and should not open or regress Sentry issues.
// long-running export" while the nightly DR D1 export holds the DB,
// and to "Network connection lost" when the D1 binding drops mid-
// query. These are transient platform unavailability errors retried
// in app code and should not open or regress Sentry issues.
//
// User-module esbuild failures and executor sandbox timeouts from
// inline workflows are similarly expected caller mistakes; see
Expand Down
Loading