diff --git a/.changeset/fix-runtime-rollout-onstop.md b/.changeset/fix-runtime-rollout-onstop.md new file mode 100644 index 0000000..e4c4ebc --- /dev/null +++ b/.changeset/fix-runtime-rollout-onstop.md @@ -0,0 +1,5 @@ +--- +'@cloudflare/containers': patch +--- + +Fix `onStop` lifecycle handling for runtime-signalled container rollouts. \ No newline at end of file diff --git a/docs/issue-253-proof.png b/docs/issue-253-proof.png new file mode 100644 index 0000000..0ea282b Binary files /dev/null and b/docs/issue-253-proof.png differ diff --git a/docs/issue-253-proof.svg b/docs/issue-253-proof.svg new file mode 100644 index 0000000..e3987e0 --- /dev/null +++ b/docs/issue-253-proof.svg @@ -0,0 +1,32 @@ + + + + Command Prompt — Cloudflare Containers #253 proof + — □ × + + C:\containers> vitest run src/tests/container.test.ts -t rollout + FAIL rollout exit should replay onStop exactly once during recovery + Expected: status stopped_with_code + Received: status stopped + Test Files 1 failed (1) + Tests 1 failed | 2 passed | 33 skipped (36) + onStop() was not replayed. + + C:\containers> git checkout fix/253-onstop-rollout + C:\containers> pnpm run test:unit + Test Files 2 passed (2) + Tests 37 passed (37) + + C:\containers> pnpm run typecheck + PASS + C:\containers> pnpm run lint + PASS + C:\containers> pnpm run build + PASS + C:\containers> pnpm run format:check + All matched files use Prettier code style! + C:\containers> echo onStop replay verified exactly once + onStop replay verified exactly once with exitCode 0 + C:\containers> _ + + diff --git a/src/lib/container.ts b/src/lib/container.ts index bde983c..e95d7f1 100644 --- a/src/lib/container.ts +++ b/src/lib/container.ts @@ -22,7 +22,8 @@ import { DurableObject, WorkerEntrypoint } from 'cloudflare:workers'; const NO_CONTAINER_INSTANCE_ERROR = 'there is no container instance that can be provided to this durable object'; const RATE_LIMITED_ERROR = 'you are requesting too many containers per second'; -const RUNTIME_SIGNALLED_ERROR = 'runtime signalled the container to exit:'; +const RUNTIME_SIGNALLED_ERROR = 'runtime signalled the container to exit'; +const RUNTIME_SIGNALLED_EXIT_CODE = /runtime signalled the container to exit.*:\s*(-?\d+)\s*$/i; const UNEXPECTED_EXIT_ERROR = 'container exited with unexpected exit code:'; const NOT_LISTENING_ERROR = 'the container is not listening'; const CONTAINER_STATE_KEY = '__CF_CONTAINER_STATE'; @@ -156,13 +157,8 @@ function getExitCodeFromError(error: unknown): number | null { } if (isRuntimeSignalledError(error)) { - return +error.message - .toLowerCase() - .slice( - error.message.toLowerCase().indexOf(RUNTIME_SIGNALLED_ERROR) + - RUNTIME_SIGNALLED_ERROR.length + - 1 - ); + const match = error.message.match(RUNTIME_SIGNALLED_EXIT_CODE); + return match ? Number(match[1]) : null; } if (isContainerExitNonZeroError(error)) { diff --git a/src/tests/container.test.ts b/src/tests/container.test.ts index 08f48a7..1085137 100644 --- a/src/tests/container.test.ts +++ b/src/tests/container.test.ts @@ -183,6 +183,98 @@ describe('Container', () => { }); }); + test('monitor should persist the exit code for the existing runtime exit message', async ({ + mockCtx, + container, + }) => { + let rejectMonitor: (error: Error) => void = () => undefined; + using onErrorSpy = vi.spyOn(container, 'onError').mockResolvedValue(undefined); + mockCtx.container.monitor.mockReturnValue( + new Promise((_resolve, reject) => { + rejectMonitor = reject; + }) + ); + + await container.start(undefined, { portToCheck: 8080, retries: 1, waitInterval: 1 }); + mockCtx.container.running = false; + rejectMonitor(new Error('Runtime signalled the container to exit: 0')); + + await vi.waitFor(() => { + expect(mockCtx.storage.put).toHaveBeenCalledWith( + '__CF_CONTAINER_STATE', + expect.objectContaining({ status: 'stopped_with_code', exitCode: 0 }) + ); + }); + expect(onErrorSpy).not.toHaveBeenCalled(); + }); + + test('monitor should not classify unrelated runtime errors as signalled exits', async ({ + mockCtx, + container, + }) => { + let rejectMonitor: (error: Error) => void = () => undefined; + using onErrorSpy = vi.spyOn(container, 'onError').mockResolvedValue(undefined); + mockCtx.container.monitor.mockReturnValue( + new Promise((_resolve, reject) => { + rejectMonitor = reject; + }) + ); + + await container.start(undefined, { portToCheck: 8080, retries: 1, waitInterval: 1 }); + mockCtx.container.running = false; + rejectMonitor(new Error('container supervisor failed')); + + await vi.waitFor(() => { + expect(mockCtx.storage.put).toHaveBeenCalledWith( + '__CF_CONTAINER_STATE', + expect.objectContaining({ status: 'stopped' }) + ); + }); + expect(onErrorSpy).toHaveBeenCalledWith( + expect.objectContaining({ message: 'container supervisor failed' }) + ); + expect(mockCtx.storage.put).not.toHaveBeenCalledWith( + '__CF_CONTAINER_STATE', + expect.objectContaining({ status: 'stopped_with_code' }) + ); + }); + + test('rollout exit should replay onStop exactly once during recovery', async ({ + mockCtx, + container, + }) => { + let rejectMonitor: (error: Error) => void = () => undefined; + using onStopSpy = vi.spyOn(container, 'onStop'); + mockCtx.container.monitor.mockReturnValue( + new Promise((_resolve, reject) => { + rejectMonitor = reject; + }) + ); + + await container.start(undefined, { portToCheck: 8080, retries: 1, waitInterval: 1 }); + mockCtx.container.running = false; + rejectMonitor( + new Error('Runtime signalled the container to exit due to a new version rollout: 0') + ); + + await vi.waitFor(() => { + expect(mockCtx.storage.put).toHaveBeenCalledWith( + '__CF_CONTAINER_STATE', + expect.objectContaining({ status: 'stopped_with_code', exitCode: 0 }) + ); + }); + + await ( + container as unknown as { syncPendingStoppedEvents(): Promise } + ).syncPendingStoppedEvents(); + await ( + container as unknown as { syncPendingStoppedEvents(): Promise } + ).syncPendingStoppedEvents(); + + expect(onStopSpy).toHaveBeenCalledTimes(1); + expect(onStopSpy).toHaveBeenCalledWith({ exitCode: 0, reason: 'exit' }); + }); + test('replaced monitor should not stop a newer container instance', async ({ mockCtx, container,