Track in-flight operations by executor lifetime, not request-cache subscribers#5360
Track in-flight operations by executor lifetime, not request-cache subscribers#5360jonreading81 wants to merge 2 commits into
Conversation
|
Hi @jonreading81! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
8b860cc to
26de1b0
Compare
…bscribers Introduce a per-environment map of operation completion promises keyed by request identifier. OperationExecutor writes an entry on construction and resolves + removes it on cancel(). Environment exposes it via getPromiseForInFlightOperation. getPendingOperationsForFragment consults it as a last-resort correlation when neither the fetch-cache subject nor the operation tracker holds a promise. This closes a fragment-to-owner correlation gap on streaming responses where the request-cache subject is drained between render passes while incremental payloads are still being processed.
f8d9575 to
1192e91
Compare
`_operationExecutions` flips to `'inactive'` while `_state === 'loading_final'` (with no pending module payloads) — before the network stream completes and `_inFlightOperationCompletions` is cleared in `cancel()`. Under incremental delivery, fragments reading during that "final-but-not-complete" window fall through to partial-data reads instead of suspending on the still-valid completion promise, defeating this correlation. Gate solely on the completion map entry's presence (via `getPromiseForInFlightOperation`) — populated in the Executor constructor and cleared atomically when `_complete` fires — which IS the correct in-flight signal for this branch. Adds a test in useFragment-WithOperationTrackerSuspense-test.js exercising the mid-stream window where isRequestActive returns false while the completion promise is still valid; the fragment must suspend on that promise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The CLA has been signed — could the bot re-check when it gets a chance? Thanks. |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Summary
Closes a fragment-to-owner correlation gap that surfaces on streaming responses when the request-cache subject is drained between Suspense render passes while the underlying
OperationExecutoris still emitting incremental payloads. In that window,getPendingOperationsForFragmentreturnsnull,useFragmentreads partial data instead of suspending, and any consumer that reads a non-null field on the missing subtree crashes the render.Introduces a per-environment registry of operation completion promises tied to
OperationExecutorlifetime, independent of the request-cache subject and its subscriber count. Behavior change is gated behind a newRelayFeatureFlags.ENABLE_IN_FLIGHT_OPERATION_CORRELATION(default off) for gradual rollout.Mechanism (before this PR)
fetchQueryDedupedcaches a per-identifier subject and tears it down when subscriber count hits 0. Under React 19 streaming SSR, that cleanup fires between render passes for a query still emitting@deferchunks — subject torn down while the executor is alive.getPromiseForActiveRequestreturnsnull(cache empty);RelayOperationTracker.getPendingOperationsAffectingOwnerreturnsnull(no other operation writes to the owner's records).getPendingOperationsForFragmentreturnsnull.useFragmentreads the partial snapshot.Discussion and trace: #5357.
What this PR does
RelayModernEnvironment— adds_inFlightOperationCompletions: Map<string, {promise, resolve}>and a publicgetPromiseForInFlightOperation(id)accessor.OperationExecutor— registers a completion promise on construction; resolves and removes it incancel().getPendingOperationsForFragment— aftergetPromiseForActiveRequestandRelayOperationTrackerboth come up null, consults the in-flight registry as a last-resort correlation. Guarded by the feature flag; the registry entry's presence IS the "in-flight" signal (populated onOperationExecutorconstruction, cleared atomically incancel()). We deliberately do NOT additionally gate onisRequestActive— that reads_operationExecutions, which flips to'inactive'while_state === 'loading_final'(no pending module payloads) BEFORE the network stream completes and the completion map is cleared. Fragments reading during that window would otherwise fall through to partial-data reads.RelayFeatureFlags— addsENABLE_IN_FLIGHT_OPERATION_CORRELATION: boolean(defaultfalse).MultiActorEnvironment/ActorSpecificEnvironment/IEnvironment/IMultiActorEnvironment— mirror the accessor for symmetry.environment.getPromiseForInFlightOperationcovering never-started / in-flight / completed; two integration tests inuseFragment-WithOperationTrackerSuspense-test.js: the first mirrors the existing suspense pattern (fragment reads missing data, executor alive, cache empty → suspends → resolves onmock.complete); the second directly asserts the gate skipsisRequestActive— with the executor still in flight butisRequestActivemocked tofalse, the fragment must still suspend on the completion promise.Design notes
getPendingOperationsForFragment, notgetPromiseForActiveRequest, so mutation-driven correlations that legitimately resolve throughRelayOperationTrackerare not short-circuited (an earlier attempt at the wrong level broke a FragmentResource plural-mutation test).OperationExecutorconstruction and removed incancel(), so lifecycle paths that already reachcancel()clean up naturally. No new leak surface; same invariant as the sibling_operationExecutionsmap.ExecuteConfiggains one field (inFlightOperationCompletions), threaded through by allenv.execute*paths — same shape as the existingoperationExecutionsparam.Test plan
yarn jest packages/relay-runtime— 2279/2279 (28 snapshots)yarn jest packages/react-relay— 1274/1274 (20 skipped)useFragment-WithOperationTrackerSuspense-test.js— 6/6 including the two new integration tests.