Skip to content

fix(plugins): cancel every in-flight request in the D1, libSQL and Trino HTTP clients - #2736

Merged
datlechin merged 2 commits into
mainfrom
fix/http-plugin-cancel-every-request
Sep 11, 2026
Merged

fix(plugins): cancel every in-flight request in the D1, libSQL and Trino HTTP clients#2736
datlechin merged 2 commits into
mainfrom
fix/http-plugin-cancel-every-request

Conversation

@datlechin

Copy link
Copy Markdown
Member

Stop on a Cloudflare D1, libSQL (Hrana over HTTP) or Trino connection cancelled whichever HTTP request started last. That was often a sidebar or autocomplete read, not the query the user stopped. Cancelling the Swift task that awaited a request did not reach the request either. The Cloudflare R2 SQL rewrite in #2030 found this same defect in its own transport. This PR fixes the three clients that shared it.

Cause

  • D1 and Hrana each kept one currentTask slot, and every send overwrote it. cancelQuery cancelled only the latest request. A request that finished cleared the slot even when it held someone else's task, so the next Stop cancelled nothing. The withTaskCancellationHandler cancelled the slot as well, so cancelling one Task could cancel a different request.
  • Trino's transport sent with dataTask and a continuation with no cancellation handler, so Task cancellation never reached the socket. The single slot was in TrinoStatementClient (_cancelled / _currentNextUri). Every new statement reset both fields, so a sidebar read that started mid-query wiped the query's cancel flag and its nextUri. When that read finished, the next cancel() sent no DELETE at all.

Fix

All three transports now use URLSession.data(for:delegate:). A per-request URLSessionTaskDelegate registers its task in urlSession(_:didCreateTask:), and a lock-protected in-flight set holds every request until its defer removes it. cancelAll() cancels each one. Cancelling the awaiting Task cancels its own URL task, which data(for:) does natively.

  • D1 and Hrana: cancelCurrentTask() becomes cancelAll(), and each driver's cancelQuery calls it. Disconnect still cancels everything through invalidateAndCancel(). A cancelled request now throws CancellationError() instead of a raw URLError -999, which DatabaseCancellationDiagnosis.isCancellation already recognises. So a sidebar read that Stop cancels reads as a cancellation, not a failure. libSQL's local sqlite3_interrupt path is untouched.
  • Trino transport: cancelAll() is added to the TrinoTransport protocol. A deinit now invalidates the session, which was leaked on every connect before. A DELETE is never tracked, because it is how a statement tells Trino to stop, and a second Stop must not cancel the first Stop's DELETE.
  • Trino statement client: each running statement has its own record, with a cancelled flag, its current nextUri and a claim-once release. cancel() flags every running statement, calls transport.cancelAll(), and sends one DELETE per statement. A statement that ends by cancellation, through cancel() or through its Task, sends its own DELETE, because data(for:) now throws before the loop's cancellation check would run.

Cancelling the initial POST is safe. Trino's QueuedStatementResource only dispatches a query on the first GET of its nextUri (submitIfNeeded), and purges one that is never polled. So a POST cancelled mid-flight never starts on the server.

Timeouts (HttpQueryTimeoutBox), auth headers, HTTP error mapping, Trino's retry and backoff, session headers and the paging loop are unchanged.

Verification

  • TrinoURLSessionTransportTests (new, 6 cases against a URLProtocol stub):
    • a round trip that carries the timeout and headers;
    • cancelAll cancelling two concurrent sends;
    • Task cancellation cancelling its request;
    • cancelAll sparing a DELETE;
    • cancel() stopping two running statements while a third starts and finishes in between, with one DELETE for each of the two;
    • Task cancellation sending a DELETE.
  • The 6 new cases pass along with the 85 existing Trino XCTest cases, and passed on 5 more runs.
  • testCancelStopsPolling also checks that cancelAll was called.
  • Mutation check: with the old TrinoStatementClient behind the new tests, the concurrent-statements test fails (requests run to the 60 s timeout, wrong DELETEs), and so does the Task-cancel test (no DELETE sent).
  • D1HttpClientCancellationTests and HranaHttpClientCancellationTests (new, 5 cases each): round trip with token and timeout, cancelAll cancelling two concurrent requests, a finished request leaving the others cancellable, Task cancellation, and disconnect. Both client files compile on their own, so project.yml adds them to the TableProTests sources. 36 of 36 pass with the existing D1 suites.
  • CloudflareD1DriverPlugin, LibSQLDriverPlugin and TrinoDriverPlugin each build with no warnings in the touched files. The AllPlugins aggregate was not run: it fails locally in the third-party OracleNIO checkout under the Xcode beta toolchain, in code this PR does not touch.
  • swiftlint --strict on the 11 changed Swift files: 0 violations. That includes four findings that were already on untouched lines of edited files: two implicit returns, one unused enumerated() and one import order.

Not covered by tests: TLS client certificates and CA pinning. The per-request delegate implements no challenge methods, so challenges still go to TrinoTLSDelegate, but the URLProtocol stub cannot exercise that. A response that lands after a DELETE can still surface as Trino's USER_CANCELED query error rather than .cancelled, as before.

No UI automation: registry plugins never load in the UI test host, and there is no D1, Turso or Trino server to talk to.

After merge, these need plugin releases: plugin-cloudflare-d1, plugin-libsql, plugin-trino.

https://claude.ai/code/session_01JKFSBk6YwDemnkbQnyc2xz

@datlechin
datlechin merged commit 1c19131 into main Sep 11, 2026
3 checks passed
@datlechin
datlechin deleted the fix/http-plugin-cancel-every-request branch September 11, 2026 14:06
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.

1 participant