Give each request its own execution stack build - #1802
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | a2ed2ab | Commit Preview URL Branch Preview URL |
Aug 28 2026, 06:09 AM |
Contributor
Cloudflare previewTorn down — the PR is closed. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | a2ed2ab | Aug 28 2026, 06:12 AM |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
Concurrent requests shared one database provider build, so a request could use a connection owned by another request and lose it when that request finished.
RhysSullivan
force-pushed
the
test/dbprovider-request-scoping
branch
from
August 28, 2026 06:08
acee781 to
a2ed2ab
Compare
RhysSullivan
marked this pull request as ready for review
August 28, 2026 06:12
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Concurrent API requests could share one database provider build, so a request could use — and lose — a connection owned by another request.
Every request gets its own database connection: opened when the request fiber's scope opens, closed when it closes. But
makeExecutionStackMiddlewarecaptures the boot fiber's context once at layer-construction time and re-applies it to every request withEffect.provideContext. A captured context also carries Effect'sCurrentMemoMap, and the merge lets the boot map overwrite the fresh per-request map the host installed.The per-request
Effect.provide(stackLayer)then memoized its build in that boot map, which every in-flight request in the isolate shares. Sequential requests still rebuilt — the memo entry is refcounted by observer and drops to zero when the request scope closes — so only overlapping requests were affected. Three concurrent requests produced one stack build, leaving two of them operating a database handle owned by a third.Two consequences follow. On Workers, the borrowing request performs I/O on a socket belonging to a different request. And when the owning request finishes, its scope finalizer ends the connection, so the borrower's next query — typically the read after a slow outbound call — fails on a connection that is already gone.
Fix
The stack is built with a request-local memoization scope (
Effect.provide(stackLayer, { local: true })) at both branches, so overlapping requests each build their own stack over their own connection. The captured context keeps doing the one job it exists for: carrying the boot-scoped services the per-request body needs.{ local: true }rather than stripping the memo map out of the captured context: it states the invariant at the call site that depends on it, it is the documented meaning of the option, and it does not depend on which memoization map happens to be ambient — so it holds for both cloud (which combines a request-scoped DB layer) and self-host (which does not).Two other per-request provider builds run under a captured context for the same stated reason — closing over the per-request socket — and had the same defect: the account provider and the admin-users provider. Both are fixed the same way. The fixed-executor middleware captures a context too, but provides only pre-built values and never builds a layer per request, so it is unaffected.
Testing
apps/cloud/src/api.request-scope.node.test.tsgains three regressions that stand the real middleware up over authenticated requests and per-request in-memory databases, so the stack layer actually builds:All three fail against the unfixed code — one build for three requests, a single handle backing every request, and an inverted release order where the slow request's own connection is released while it keeps working on the fast request's. All pass after the change, over five consecutive runs of the file.
bun run typecheckis green. Cloud e2estorage-error-report-shapeandconnections-credentialspass unchanged.