fix(rivetkit): stall stop handler until start completes#4192
fix(rivetkit): stall stop handler until start completes#4192MasterPtato wants to merge 1 commit intomainfrom
Conversation
|
🚅 Deployed to the rivet-pr-4192 environment in rivet-frontend
|
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR Review: fix(rivetkit): stall stop handler until start completesSummaryThis PR fixes a race condition in the engine actor driver where a stop signal could arrive before the actor's start/initialization was complete, potentially leaving the system in an inconsistent state. It also adds tests for connection error serialization. Core Fix:
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| this.#runnerStarted.resolve(undefined); | ||
| }, | ||
| onDisconnected: (_code, _reason) => {}, | ||
| onDisconnected: (_code, _reason) => { }, |
There was a problem hiding this comment.
Remove spaces inside the empty function braces to comply with Biome linter formatting rules. Change onDisconnected: (_code, _reason) => { }, to onDisconnected: (_code, _reason) => {},
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
| return streamSSE(c, async (stream) => { | ||
| // NOTE: onAbort does not work reliably | ||
| stream.onAbort(() => {}); | ||
| stream.onAbort(() => { }); |
There was a problem hiding this comment.
Remove spaces inside the empty function braces to comply with Biome linter formatting rules. Change stream.onAbort(() => { }); to stream.onAbort(() => {});
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
| import { actor } from "rivetkit"; | ||
|
|
||
| /** | ||
| * Actor designed to test start/stop race conditions. | ||
| * Has a slow initialization to make race conditions easier to trigger. | ||
| */ | ||
| export const startStopRaceActor = actor({ | ||
| state: { | ||
| initialized: false, | ||
| startTime: 0, | ||
| destroyCalled: false, | ||
| startCompleted: false, | ||
| }, | ||
| onWake: async (c) => { | ||
| c.state.startTime = Date.now(); | ||
|
|
||
| // Simulate slow initialization to create window for race condition | ||
| await new Promise((resolve) => setTimeout(resolve, 100)); | ||
|
|
||
| c.state.initialized = true; | ||
| c.state.startCompleted = true; | ||
| }, | ||
| onDestroy: (c) => { | ||
| c.state.destroyCalled = true; | ||
| // Don't save state here - the actor framework will save it automatically | ||
| }, | ||
| actions: { | ||
| getState: (c) => { | ||
| return { | ||
| initialized: c.state.initialized, | ||
| startTime: c.state.startTime, | ||
| destroyCalled: c.state.destroyCalled, | ||
| startCompleted: c.state.startCompleted, | ||
| }; | ||
| }, | ||
| ping: (c) => { | ||
| return "pong"; | ||
| }, | ||
| destroy: (c) => { | ||
| c.destroy(); | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| /** | ||
| * Observer actor to track lifecycle events from other actors | ||
| */ | ||
| export const lifecycleObserver = actor({ | ||
| state: { | ||
| events: [] as Array<{ | ||
| actorKey: string; | ||
| event: string; | ||
| timestamp: number; | ||
| }>, | ||
| }, | ||
| actions: { | ||
| recordEvent: (c, params: { actorKey: string; event: string }) => { | ||
| c.state.events.push({ | ||
| actorKey: params.actorKey, | ||
| event: params.event, | ||
| timestamp: Date.now(), | ||
| }); | ||
| }, | ||
| getEvents: (c) => { | ||
| return c.state.events; | ||
| }, | ||
| clearEvents: (c) => { | ||
| c.state.events = []; | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Run the Biome formatter on this file to ensure proper formatting and sorted imports. The file is new and likely has formatting issues that don't match the project's style guide.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
MasterPtato
force-pushed
the
02-13-fix_rivetkit_stall_stop_handler_until_start_completes
branch
from
1b0d832 to
0f6bab1
Compare
| import { startStopRaceActor, lifecycleObserver } from "./start-stop-race"; | ||
| import { connErrorSerializationActor } from "./conn-error-serialization"; |
There was a problem hiding this comment.
Imports should be sorted alphabetically. Consider reordering these imports to maintain consistent sorting.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
| import { runActorConnHibernationTests } from "./tests/actor-conn-hibernation"; | ||
| import { runActorConnStateTests } from "./tests/actor-conn-state"; | ||
| import { runActorDbTests } from "./tests/actor-db"; | ||
| import { runConnErrorSerializationTests } from "./tests/conn-error-serialization"; |
There was a problem hiding this comment.
This import should be sorted alphabetically with the other imports to maintain consistent ordering.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
MasterPtato
force-pushed
the
02-13-fix_rivetkit_stall_stop_handler_until_start_completes
branch
from
0f6bab1 to
8f7b569
Compare
1 participant
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: