Skip to content

Fix onStop handling for container rollouts - #255

Open
SagsMan wants to merge 4 commits into
cloudflare:mainfrom
SagsMan:fix/253-onstop-rollout
Open

SagsMan wants to merge 4 commits into
cloudflare:mainfrom
SagsMan:fix/253-onstop-rollout

Conversation

@SagsMan

@SagsMan SagsMan commented Sep 16, 2026 •

Copy link
Copy Markdown

1. Summary

This patch fixes container lifecycle handling when Cloudflare replaces a container during an image rollout.

  1. Recognize both the existing runtime exit message and the rollout-specific message.
  2. Preserve the numeric exit code so stopped-event recovery can replay lifecycle hooks.
  3. Add regression coverage for message parsing, unrelated runtime errors, and one-time onStop() replay.

2. Root cause

The SDK only matched this message shape:

Runtime signalled the container to exit: 0

During a rollout, the runtime emits:

Runtime signalled the container to exit due to a new version rollout: 0

Because the rollout message contains additional text before the colon, the SDK did not recognize it as a runtime-signalled exit. The monitor then stored the container as plain stopped instead of stopped_with_code.

syncPendingStoppedEvents() does not replay onStop() for the plain stopped state, so cleanup and persistence hooks were skipped during recovery.

3. Fix

The runtime-exit matcher now uses the shared message prefix and extracts the terminal numeric exit code from either message format. This keeps the existing behavior intact while handling rollout replacements correctly.

4. Validation

  1. pnpm run test:unit — 37 tests passed
  2. pnpm run typecheck — passed
  3. pnpm run lint — passed
  4. pnpm run build — passed
  5. pnpm run format:check — passed
  6. git diff --check — passed

The regression tests verify that:

  • the existing runtime exit message still produces stopped_with_code;
  • the rollout message produces stopped_with_code with exit code 0;
  • unrelated runtime errors are not classified as signalled exits; and
  • onStop() is replayed exactly once.

5. Execution proof

The image below shows the fresh before-and-after unit-test evidence. The parent revision fails to recognize the rollout message and skips onStop(). The PR branch passes the targeted regression tests and the full unit suite.

Issue #253 before and after proof

Open the proof image directly

Fixes #253

@SagsMan
SagsMan requested a review from a team as a code owner September 16, 2026 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

onStop() never fires when a rollout replaces a container: exit-reason substring misses the runtime's rollout message

1 participant