cloudflare / cloudflare/containers
onStop() never fires when a rollout replaces a container: exit-reason substring misses the runtime's rollout message
- Dominant language
- TypeScript
- Stars
- 270
- Forks
- 42
- Avg merge
- 1d 5h
- Merged PRs (30d)
- 4
Description
### Version
`@cloudflare/containers@0.3.7` (current latest)
### Summary
`onStop()` is never called when a container is replaced by a **deploy image rollout** — the single most common replacement cause. The exit-reason detection matches on a message substring that the runtime's own rollout message does not contain, so the container settles in a state that `syncPendingStoppedEvents` does not replay.
Any cleanup, cookie/session persistence, or telemetry hung off `onStop()` silently never runs for these stops. There is no error and no warning; the hook simply doesn't fire.
### The mismatch
The constant carries a trailing colon immediately after `exit` (`dist/lib/container.js:10`):
```js
const RUNTIME_SIGNALLED_ERROR = 'runtime signalled the container to exit:';
```
The message the runtime actually produces on an image rollout inserts text before that colon. Observed verbatim in our Worker logs:
```
Container error: Error: Runtime signalled the container to exit due to a new version rollout: 0
```
`isErrorOfType` is a lowercased `includes`, so `isRuntimeSignalledError()` returns `false` for the rollout variant.
### The resulting path
1. `isRuntimeSignalledError` → `false`, so `getExitCodeFromError` returns `null`.
2. The monitor's rejection handler therefore falls through to `state.setStopped()` instead of `setStoppedWithCode(exitCode)`, leaving status `'stopped'`.
3. `syncPendingStoppedEvents` (`dist/lib/container.js:1594-1604`) replays only `'healthy' | 'running' | 'stopped_with_code'`:
```js
async syncPendingStoppedEvents() {
const state = await this.state.getState();
if (!this.container.running && (state.status === 'healthy' || state.status === 'running')) {
await this.callOnStop({ exitCode: 0, reason: 'exit' }, state);
return;
}
if (!this.container.running && state.status === 'stopped_with_code') {
await this.callOnStop({ exitCode: state.exitCode ?? 0, reason: 'exit' }, state);
return;
}
}
```
`'stopped'` matches neither branch, so `onStop()` is never delivered for that container.
Note that `onError()` *is* called on this path, so a subclass overriding `onError` sees the failure — but `onStop` consumers do not.
### Reproduction
1. A Durable Object with a `Container` subclass that overrides `onStop()`.
2. Start a container and keep it connected past `rollout_active_grace_period`.
3. `wrangler deploy` with a changed image so a rollout replaces that instance.
4. `onStop()` does not fire. `onError()` does.
### Suggested fix
Either widen the match so the rollout variant is recognised — dropping the trailing colon from the constant is enough, since the two known forms are `...to exit: ` and `...to exit due to a new version rollout: ` — or, more robustly, key the exit reason off a structured signal rather than message text.
As defence in depth, `syncPendingStoppedEvents` could also replay a plain `'stopped'` state (with an unknown exit code), so an unrecognised exit reason degrades to "onStop fired without a code" rather than "onStop never fires".
Contributor guide
Research direction
Start in dist/lib/container.js at RUNTIME_SIGNALLED_ERROR and isRuntimeSignalledError(), then trace the monitor rejection path into syncPendingStoppedEvents. Reproduce a deploy-image rollout with an onStop() and onError() override, and verify that the rollout message is recognised and onStop() fires with the expected or explicitly unknown exit code.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100