Skip to content

Commit 3e6868c

Browse files
Prashansa-Kclaude
andcommitted
fix(cli): classify the functions serve failure paths from real state
`streamContainerLogs` decided the daemon was unreachable, and whether a stream failure was retriable, by substring-matching the tail of the container's own logs — arbitrary user function output. Both decisions now come from a follow-up `docker container inspect`, which is Docker's output alone, so a function that logs "no such container" can no longer flip a fatal failure into an endless retry. Reuse the shared `inspectContainerState` rather than a same-named private copy that parsed a different wire format and failed with bare `Error`s: a container removed between the log stream ending and the inspect now ends the session instead of reporting raw Docker text as an unclassified failure. Supervisor teardown codes (129/130/131/143) end the session successfully with their own message; real crash signals still fail as `runtime_crash`. A clean container exit prints a line distinct from the user-initiated shutdown so scrollback can tell the two apart. Re-attaches resume with `--since` instead of replaying the whole log history, are capped, and run in their own scope so child handles no longer accumulate for the session. The local-database pre-flight failed with bare `Error`s, so the check that runs on every invocation reported no category at all. It now carries tagged identities: a missing stack suggests `supabase start`, an unreachable daemon suggests starting Docker. The shutdown grace period only guarded the log-stream race, leaving the startup race — where a cold image pull is most likely to meet a Ctrl-C — to lose the same coin flip. Both races now share one helper, and the grace and retry timers are injectable so their tests no longer race a real clock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 3d093b9 commit 3e6868c

10 files changed

Lines changed: 771 additions & 258 deletions

File tree

apps/cli/src/command-internal/docker-lifecycle.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ export const inspectContainerState = (spawner: Spawner, containerId: string) =>
248248
function parseContainerState(stdout: string): {
249249
readonly running: boolean;
250250
readonly status: string;
251+
readonly exitCode: number;
251252
readonly health?: string;
252253
} {
253254
const trimmed = stdout.trim();
@@ -263,12 +264,13 @@ function parseContainerState(stdout: string): {
263264
// status text; `status` is kept only for error message text.
264265
const status = typeof state["Status"] === "string" ? state["Status"] : "";
265266
const running = state["Running"] === true;
267+
const exitCode = typeof state["ExitCode"] === "number" ? state["ExitCode"] : 0;
266268
const health = state["Health"];
267269
const healthStatus =
268270
isJsonRecord(health) && typeof health["Status"] === "string" ? health["Status"] : undefined;
269271
return healthStatus !== undefined
270-
? { running, status, health: healthStatus }
271-
: { running, status };
272+
? { running, status, exitCode, health: healthStatus }
273+
: { running, status, exitCode };
272274
}
273275

274276
function isJsonRecord(value: unknown): value is { readonly [key: string]: unknown } {

apps/cli/src/command-internal/docker-lifecycle.unit.test.ts

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -220,12 +220,18 @@ describe("inspectContainerState", () => {
220220
stdout: JSON.stringify({
221221
Status: "running",
222222
Running: true,
223+
ExitCode: 0,
223224
Health: { Status: "healthy" },
224225
}),
225226
});
226227
return inspectContainerState(mock.spawner, "supabase_db_my-app").pipe(
227228
Effect.map((state) => {
228-
expect(state).toEqual({ running: true, status: "running", health: "healthy" });
229+
expect(state).toEqual({
230+
running: true,
231+
status: "running",
232+
exitCode: 0,
233+
health: "healthy",
234+
});
229235
expect(mock.spawned).toEqual([
230236
{
231237
command: "docker",
@@ -237,30 +243,36 @@ describe("inspectContainerState", () => {
237243
});
238244

239245
it.live("parses a running container with no health check configured", () => {
240-
const mock = mockSpawner({ stdout: JSON.stringify({ Status: "running", Running: true }) });
246+
const mock = mockSpawner({
247+
stdout: JSON.stringify({ Status: "running", Running: true, ExitCode: 0 }),
248+
});
241249
return inspectContainerState(mock.spawner, "supabase_kong_my-app").pipe(
242250
Effect.map((state) => {
243-
expect(state).toEqual({ running: true, status: "running" });
251+
expect(state).toEqual({ running: true, status: "running", exitCode: 0 });
244252
}),
245253
);
246254
});
247255

248256
it.live("parses a stopped/exited container", () => {
249-
const mock = mockSpawner({ stdout: JSON.stringify({ Status: "exited", Running: false }) });
257+
const mock = mockSpawner({
258+
stdout: JSON.stringify({ Status: "exited", Running: false, ExitCode: 1 }),
259+
});
250260
return inspectContainerState(mock.spawner, "supabase_kong_my-app").pipe(
251261
Effect.map((state) => {
252-
expect(state).toEqual({ running: false, status: "exited" });
262+
expect(state).toEqual({ running: false, status: "exited", exitCode: 1 });
253263
}),
254264
);
255265
});
256266

257267
it.live(
258268
"treats a paused/restarting container as running, matching Go's boolean-based gate",
259269
() => {
260-
const mock = mockSpawner({ stdout: JSON.stringify({ Status: "paused", Running: true }) });
270+
const mock = mockSpawner({
271+
stdout: JSON.stringify({ Status: "paused", Running: true, ExitCode: 0 }),
272+
});
261273
return inspectContainerState(mock.spawner, "supabase_db_my-app").pipe(
262274
Effect.map((state) => {
263-
expect(state).toEqual({ running: true, status: "paused" });
275+
expect(state).toEqual({ running: true, status: "paused", exitCode: 0 });
264276
}),
265277
);
266278
},
@@ -322,7 +334,7 @@ describe("inspectContainerState", () => {
322334
const mock = mockSpawner({ stdout: "" });
323335
return inspectContainerState(mock.spawner, "supabase_db_my-app").pipe(
324336
Effect.map((state) => {
325-
expect(state).toEqual({ running: false, status: "" });
337+
expect(state).toEqual({ running: false, status: "", exitCode: 0 });
326338
}),
327339
);
328340
});
@@ -331,7 +343,7 @@ describe("inspectContainerState", () => {
331343
const mock = mockSpawner({ stdout: "null" });
332344
return inspectContainerState(mock.spawner, "supabase_db_my-app").pipe(
333345
Effect.map((state) => {
334-
expect(state).toEqual({ running: false, status: "" });
346+
expect(state).toEqual({ running: false, status: "", exitCode: 0 });
335347
}),
336348
);
337349
});

apps/cli/src/command-internal/docker-suggest.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import { containerRuntimeNotFoundMessage } from "./container-cli.ts";
44
export const SUGGEST_DOCKER_INSTALL =
55
"Docker Desktop is a prerequisite for local development. Follow the official docs to install: https://docs.docker.com/desktop";
66

7+
/** Remediation for a daemon that answered earlier in the session and then went away, as opposed to `SUGGEST_DOCKER_INSTALL`'s missing-binary case. */
8+
export const SUGGEST_DOCKER_START =
9+
"Docker is no longer reachable. Start Docker, then rerun `supabase functions serve`.";
10+
711
/**
812
* Whether a container-CLI stderr indicates the daemon is unreachable. Matches the docker/podman
913
* "cannot connect"/"is the docker daemon running" messages, a socket permission-denied message,

apps/cli/src/commands/functions/serve/SIDE_EFFECTS.md

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,17 @@ back to local keys. No scheme/host validation is performed on the discovered URL
6161

6262
## Exit Codes
6363

64-
| Code | Condition |
65-
| ---- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
66-
| `0` | clean shutdown after `SIGINT`, `SIGTERM`, or stdin close |
67-
| `0` | the edge-runtime container stops on its own with exit code `0` |
68-
| `1` | local DB container is not running, or the Docker daemon is unreachable (surfaces from the DB inspect as `failed to inspect service: …` plus the Docker Desktop install suggestion) |
69-
| `1` | invalid inspect flag combination, or a `Config.Validate` failure anywhere in `config.toml` (not just project/auth config) |
70-
| `1` | env file, signing key, import map, or function bind resolution failure |
71-
| `1` | edge-runtime container startup, log streaming, or restart loop failure |
64+
| Code | Condition |
65+
| ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
66+
| `0` | clean shutdown after `SIGINT` or `SIGTERM` |
67+
| `0` | the edge-runtime container stops on its own with exit code `0` |
68+
| `0` | the edge-runtime container is torn down by an external supervisor — exit `129`/`130`/`131`/`143` (`SIGHUP`/`SIGINT`/`SIGQUIT`/`SIGTERM`), e.g. `supabase stop` run in another terminal |
69+
| `0` | the edge-runtime container is already gone by the time a follow-up `docker container inspect` runs after the log stream ended |
70+
| `0` | an edge-runtime startup failure or log-stream failure lands within the shutdown grace period (~50ms) of a `SIGINT`/`SIGTERM` |
71+
| `1` | local DB container is not running, or the Docker daemon is unreachable (surfaces from the DB inspect as `failed to inspect service: …` plus the Docker Desktop install suggestion) |
72+
| `1` | invalid inspect flag combination, or a `Config.Validate` failure anywhere in `config.toml` (not just project/auth config) |
73+
| `1` | env file, signing key, import map, or function bind resolution failure |
74+
| `1` | edge-runtime container startup, log streaming, or restart loop failure — including the edge-runtime container crashing with any exit code other than `0`, `137`, or `129`/`130`/`131`/`143` |
7275

7376
## Telemetry Events Fired
7477

@@ -86,7 +89,10 @@ Writes lifecycle text to stderr / stdout while the command is running:
8689
- `Skipped serving Function: <slug>` for disabled functions
8790
- `File change detected: <path> (<op>)` when a watched file triggers a restart
8891
- live `docker logs -f --timestamps` output from the edge-runtime container
89-
- `Stopped serving supabase/functions` on clean shutdown
92+
- `Stopped serving supabase/functions` on a user-initiated shutdown (`SIGINT`/`SIGTERM`)
93+
- `Edge Runtime exited (code 0). Stopped serving supabase/functions` when the container stops on its own with exit code `0`
94+
- `Edge Runtime container stopped (exit <code>). Stopped serving supabase/functions` when an external supervisor tears the container down (exit `129`/`130`/`131`/`143`)
95+
- `Edge Runtime container is no longer available. Stopped serving supabase/functions` when the container is already gone by the time a follow-up inspect runs
9096

9197
### `--output-format json`
9298

@@ -115,7 +121,9 @@ Long-running raw log / error events only; there is no terminal `result` event on
115121
- Config, project dotenv discovery, and function discovery all resolve from `<workdir>` with no ancestor search (CLI-2285), so they can never disagree.
116122
- Before each container (re)start, resolves the edge-runtime image through the same registry-candidate pull-with-retry every native `functions` Docker path uses: `docker image inspect <candidate>` (ECR, then GHCR, then Docker Hub) to check the local cache, then `docker pull <candidate>` with 2 retries (4s/8s backoff) on a miss, after `assertLocalDbRunning` — resolving it earlier would hijack the down-daemon error message that DB-inspect step is responsible for producing.
117123
- Runs the full `Config.Validate` pipeline (`resolveLocalConfigValues`, same one `start`/`stop`/`status` use) on every startup/restart, before `assertLocalDbRunning` — an invalid config now fails `serve` up front even for fields this command never otherwise reads (e.g. a bad `db.major_version` or malformed auth hook).
118-
- A container crash terminates the command with a non-zero exit; a container that stops on its own with exit code `0` ends the command successfully instead. Only a watched-file change restarts the container — neither outcome is ever auto-restarted.
124+
- A container that stops on its own with exit code `0`, or that is torn down by an external supervisor (exit `129`/`130`/`131`/`143`, e.g. `supabase stop` in another terminal), or that is already gone by the time a follow-up inspect runs, all end the command successfully — each prints its own distinct line (see Output above) rather than the user-initiated `Stopped serving …` line, so scrollback can tell "I stopped it" from "the runtime walked out" or "a supervisor tore it down". In a `functions serve &` CI step this means a runtime that exits on its own does not fail the step; the distinct message is the only signal, and a downstream failure otherwise only surfaces later as connection-refused. Exit `137` (SIGKILL, e.g. an OOM kill) is retried by re-attaching to the log stream rather than failing the command. Any other non-zero container exit fails the command; the error message includes the container id. Only a watched-file change restarts the container itself — none of these outcomes ever restart it.
125+
- A `docker logs -f` re-attach (the daemon can close the stream while the container keeps running) resumes with `--since <last forwarded log timestamp>` instead of replaying the full log history, and is capped at 5 consecutive re-attaches that forward no new output; exceeding the cap fails the command with a tagged error instead of looping forever.
126+
- On the log-stream path, the Docker-daemon-unreachable classification comes from a follow-up `docker container inspect` failure, not from `docker logs -f`'s own stderr text.
119127
- The worker bootstrap template (`serve.main.ts`) is bundled into a single self-contained module with `jose` and the local path/status helpers inlined, so the edge-runtime worker boots without any network access (supabase/supabase#45570). The bundle is embedded at build time for shipped binaries and produced on demand (esbuild) when running from source. It is delivered into the created (not yet started) container as a `docker cp` stdin tar archive at `/root/index.ts` — never a single-file host bind mount, which materializes as an empty directory on daemons that cannot see the client's filesystem (remote `DOCKER_HOST`/Docker-context daemons, podman machines) and breaks bring-up with edge-runtime's "failed to determine entrypoint" (supabase/cli#6254). Only this bootstrap template is daemon-independent: user function sources, import maps, static files, and the multiline-env script directory (present only when an env value contains a newline) still arrive by host bind mounts, so they require a daemon that can see the project directory.
120128
- The aggregated bind mount list is pruned before `docker create`: a bind is dropped when another bind of the same mode already supplies the same content at the same container path — a file bind nested inside an already-bound read-only package directory would otherwise make the bootstrap `docker cp` fail with `destination "<container>:/" must be a directory` (supabase/supabase#50088). Pruned paths remain visible in the container through their covering parent mounts; the `--workdir` gate and the file-watch set are computed from the unpruned aggregate.
121129
- Existing local values declared under an import map's `scopes` are explicit read-only Docker mounts and may resolve outside the nearest Git root; each distinct out-of-root host path prints one `WARN` during bring-up, deduplicated across Functions sharing an import map. Such out-of-root mounts are excluded from the file-watch set per Function, so a scope target contributes no watch root of its own and cannot enlarge or destabilise the watcher; a path that another Function reaches through its ordinary binds is still watched. Other file-valued binds are watched through their immediate parent non-recursively, while directory binds remain recursive. Missing targets retain serve's existing skip behavior.

0 commit comments

Comments
 (0)