fix(#595): report stalled checkpoint flushes and trace-tag stream errors - #599
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe idle client now generates per-flush trace identifiers, detects repeated unanswered checkpoint flushes, broadcasts stall telemetry through the worker, stores it in idle state, and reports trace-tagged checkpoint errors from the web game mount. ChangesCheckpoint flush telemetry
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/routes/-game/game-simulation-mount.test.tsx`:
- Around line 73-77: Replace the inline checkpointFlushStall object in the test
with the package’s faker-defaulted create-mock-* factory, overriding only
activityID, reason, and traceID at the call site. Preserve the existing values
for those three fields and remove the manually constructed domain state object.
- Around line 69-88: Update the test around GameSimulationMount to assert the
Sentry capture side effect when checkpointFlushStall is present. Verify the
captured exception contains the activity/reason message and that its telemetry
tags include traceID set to 'trace_1', while preserving the existing
empty-render assertion.
In `@libs/game/idle-client/src/state/set-checkpoint-flush-stall.test.ts`:
- Around line 5-23: Restore the process-global Zustand state after the tests
that mutate it: in
libs/game/idle-client/src/state/set-checkpoint-flush-stall.test.ts lines 5-23,
clean up checkpointFlushStall and rewardSlotLedger after each test; in
libs/game/idle-client/src/worker/use-simulation-worker.test.ts lines 147-171,
clean up checkpointFlushStall after the worker-message test. Use the existing
onTestFinished() cleanup mechanism and restore each field to its prior baseline.
In `@libs/game/idle-client/src/submission/constants.ts`:
- Around line 13-18: Update the comments describing the flush-stall threshold
and streak in libs/game/idle-client/src/submission/constants.ts:13-18 to define
the streak as consecutive non-defined failures, with success or defined contract
responses resetting it; in
libs/game/idle-client/src/submission/create-checkpoint-submitter.ts:73-79,
replace “answered request” with “success or defined contract response”; and at
:103-105, replace “unanswered flushes” with “non-defined flush failures.”
In `@libs/game/idle-client/src/submission/create-activity-service-client.ts`:
- Around line 13-17: Add a Bun regression test around the RPCLink fetch boundary
used by the activity service client, covering both cases: a call context with a
per-call traceparent must send that value as the outbound traceparent header,
while a call without traceparent must omit the header. Preserve the existing
headers behavior in the RPCLink configuration.
In `@libs/game/idle-client/src/submission/create-checkpoint-submitter.test.ts`:
- Around line 553-558: Replace the inline ActivitySubmissionContext object
passed to registerActivity in each affected test site with the package’s
faker-defaulted create-mock-* factory, overriding only activityID, appendedHead,
lastHash, and startChainIndex as asserted. Apply the same change to the
additional occurrences around the other registerActivity calls, preserving each
test’s existing field values.
- Around line 618-650: Extend the test around submit and traceparent capture to
perform two flushes: make the first request fail non-terminally, then make the
second fail with CHECKPOINT_INVALID. Assert two traceparent headers are captured
and differ, and verify onInvalid is called with the second request’s trace ID
while preserving the existing format and activity assertions.
In
`@libs/game/idle-client/src/worker/create-checkpoint-flush-stalled-message.test.ts`:
- Around line 8-12: Update the expected message in the checkpoint flush stalled
contract test to assert the literal discriminator value
'checkpoint_flush_stalled' instead of WorkerMessageType.CheckpointFlushStalled,
and remove the now-unused WorkerMessageType import.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 68bb36d9-e1e7-4ad8-a253-03d0ea8c0870
📒 Files selected for processing (25)
apps/web/src/lib/idle/use-idle-worker-handle.tsapps/web/src/routes/-game/game-simulation-mount.test.tsxapps/web/src/routes/-game/game-simulation-mount.tsxapps/web/src/test-utils/idle-worker-handle-holder.tslibs/game/idle-client/src/index.tslibs/game/idle-client/src/state/create-sync-slice.test.tslibs/game/idle-client/src/state/create-sync-slice.tslibs/game/idle-client/src/state/set-checkpoint-flush-stall.test.tslibs/game/idle-client/src/state/set-checkpoint-flush-stall.tslibs/game/idle-client/src/state/use-checkpoint-flush-stall.tslibs/game/idle-client/src/submission/constants.tslibs/game/idle-client/src/submission/create-activity-service-client.tslibs/game/idle-client/src/submission/create-checkpoint-submitter.test.tslibs/game/idle-client/src/submission/create-checkpoint-submitter.tslibs/game/idle-client/src/submission/create-traceparent.test.tslibs/game/idle-client/src/submission/create-traceparent.tslibs/game/idle-client/src/submission/types.tslibs/game/idle-client/src/types.tslibs/game/idle-client/src/worker/create-checkpoint-flush-stalled-message.test.tslibs/game/idle-client/src/worker/create-checkpoint-flush-stalled-message.tslibs/game/idle-client/src/worker/create-checkpoint-stream-invalid-message.test.tslibs/game/idle-client/src/worker/create-checkpoint-stream-invalid-message.tslibs/game/idle-client/src/worker/create-worker-runtime.tslibs/game/idle-client/src/worker/use-simulation-worker.test.tslibs/game/idle-client/src/worker/use-simulation-worker.ts
There was a problem hiding this comment.
All reported issues were addressed across 25 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…telemetry # Conflicts: # apps/web/src/lib/idle/use-idle-worker-handle.ts # apps/web/src/routes/-game/game-simulation-mount.tsx # apps/web/src/test-utils/idle-worker-handle-holder.ts # libs/game/idle-client/src/index.ts # libs/game/idle-client/src/submission/constants.ts # libs/game/idle-client/src/submission/create-checkpoint-submitter.test.ts # libs/game/idle-client/src/submission/create-checkpoint-submitter.ts # libs/game/idle-client/src/types.ts # libs/game/idle-client/src/worker/create-worker-runtime.ts
Description
Closes #595. Closes #596.
Checkpoint-flush faults now reach the error backend with a trace id the service side shares. The submitter mints a W3C
traceparentper flush, and a streak of three unanswered flushes reports a stall without stopping the stream.onFlushStalledonce per streak; any answered request (success or defined error) resets it, and the queue keeps retrying untouched.CHECKPOINT_INVALIDrejections now carry the rejecting request's trace id through the worker message, store, and the game mount's Sentry capture (traceIDtag, matching the service-side tag).traceparent, so the service continues the worker's trace instead of minting its own.Testing
bun run typecheckpassesbun run testpassesbun run lintpasses