Skip to content

Retry draft lookup before reporting "draft not found" on send (MAILSPRING-CLIENT-T) - #2848

Open
bengotow wants to merge 1 commit into
masterfrom
claude/awesome-ritchie-gi1ifq
Open

bengotow wants to merge 1 commit into
masterfrom
claude/awesome-ritchie-gi1ifq

Conversation

@bengotow

@bengotow bengotow commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

What I observed in Sentry

MAILSPRING-CLIENT-T is the highest-impact unresolved issue on release 1.21.1-ae68e881 (342 users total, 1163 events, still firing today). It's Error: Could not find draft after finalizing session for sending., thrown from DraftStore._onUnexpectedNotFoundDuringSend in app/src/flux/stores/draft-store.ts.

This exact issue was the subject of #2727, merged into master at commit ae68e881 — the same commit this release build was cut from. That PR's description proposed three fixes, including "retry the post-commit database query once with a 150ms delay" to absorb a transaction-ordering race. But diffing the actual PR only shows two of the three changes landing: the diagnostic extra payload and a TaskQueue array-leak fix. The retry logic described in the PR body was never actually implemented in the diff. That explains why the issue's event rate is unchanged after that "fix" shipped — nothing about the retrieval logic itself changed.

Root cause

DraftStore._onSendDraft awaits session.changes.commit(), which calls DraftEditingSession.changeSetCommit(). That queues a SyncbackDraftTask and awaits TaskQueue.waitForPerformLocal(task). waitForPerformLocal resolves as soon as the task's status leaves 'local'Task.hasRunLocally() returns true for any non-'local' status, including 'cancelled' (app/src/flux/tasks/task.ts:56).

The sync engine (C++) can commit the task-status transition and the draft row write in separate SQLite transactions. If the status-update transaction lands first, waitForPerformLocal resolves and _onSendDraft immediately queries DatabaseStore.findBy({ headerMessageId, draft: true }) — which can return null even though the draft row write is about to land a moment later. The result: a real user's draft that saved successfully throws a scary "could not be found" dialog and gets reported to Sentry.

Fix

In app/src/flux/stores/draft-store.ts, when the post-commit DatabaseStore.findBy lookup returns null, wait 150ms and retry the same query once before giving up and calling _onUnexpectedNotFoundDuringSend. This is long enough to absorb the transaction-ordering race (the sync engine's own change-notification throttle is itself 150ms) while being imperceptible on the happy path, where it's never hit. I also record whether the retry succeeded (diagnostics.foundOnRetry) so if the issue keeps recurring after this, Sentry will make it obvious whether the race is what's left or something else is going on.

Test plan

  • npm run lint passes on the changed file
  • tsc --noEmit shows no new errors introduced by this change (pre-existing errors in the tree are from unrelated missing optional native/type dependencies in this sandboxed environment)
  • Compose and send a draft normally — verify send still succeeds without a perceptible delay
  • Simulate the race (e.g. rapid send + concurrent draft mutation that could cancel the SyncbackDraftTask) and verify the retry resolves it without surfacing the error dialog

Fixes MAILSPRING-CLIENT-T

🤖 Generated with Claude Code

https://claude.ai/code/session_01SNN84RVmDKgi4UCtiGSKhP


Generated by Claude Code

MAILSPRING-CLIENT-T has kept firing at the same rate (342 users, 1163
events) since PR #2727 landed in this exact release (1.21.1-ae68e881).
That PR's description described a retry-with-delay fix for the
transaction-ordering race between the SyncbackDraftTask status update
and the draft row's own SQLite write, but the actual diff only added
diagnostics and a TaskQueue leak fix — the retry was never implemented.

waitForPerformLocal() resolves as soon as the task leaves the 'local'
status, including when the sync engine cancels it, which can race
ahead of the draft row commit. Retry the DatabaseStore.findBy lookup
once after a short delay to absorb that race before surfacing the
error dialog and reporting to Sentry.

Fixes MAILSPRING-CLIENT-T

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SNN84RVmDKgi4UCtiGSKhP
@indent-staging

indent-staging Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Important

Indent Zero has shut down and no longer reviews pull requests.
To get this pull request reviewed by Indent instead:

  1. Sign up for Indent
  2. Install Indent on your repositories
  3. Turn on code review
  4. Comment @indent on this pull request

Step 4 is only needed for pull requests that were already open when you switched. After that, Indent reviews new pull requests on its own.

To stop this notice, turn PR reviews off in Indent Zero.

@indent

indent Bot commented Sep 3, 2026

Copy link
Copy Markdown
PR Summary

Adds a one-shot retry to DraftStore._onSendDraft to fix Sentry issue MAILSPRING-CLIENT-T ("Could not find draft after finalizing session for sending"), the highest-impact unresolved crash on release 1.21.1. The prior PR #2727 described this retry but never actually implemented it in its diff; this PR lands it.

  • When the post-commit DatabaseStore.findBy({ headerMessageId, draft: true }) returns null, wait 150ms and retry the identical query once before reporting failure, absorbing a sync-engine race where the SyncbackDraftTask status transition commits (making waitForPerformLocal resolve) before the draft row's own SQLite transaction lands.
  • Records diagnostics.foundOnRetry and updates the failure label to ...after commit (and retry) in the Sentry report.
  • The retry runs only on the failure branch, so the happy path adds no latency; the 150ms cost applies only when the first lookup fails.

Issues

No issues found.

CI Checks

All CI checks passed on 2536a43.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants