From f0377e775926c4081894f8d754ddab28d95c7912 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Sirois Date: Tue, 4 Aug 2026 00:53:10 -0300 Subject: [PATCH] fix(remote): report what pg_restore printed when schema sync fails pipeTo already reads the restore's stderr and re-emits it, but the only subscriber forwards to a websocket that CI has no client for. A failed job therefore reported an exit code while pg_restore had already named the extension or collation it could not create. Closes Query-Doctor/Site#3836 --- src/remote/remote.ts | 18 ++++++++++--- src/sync/command-failure.test.ts | 46 ++++++++++++++++++++++++++++++++ src/sync/command-failure.ts | 37 +++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 src/sync/command-failure.test.ts create mode 100644 src/sync/command-failure.ts diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 4ced906..95c1c8e 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -12,6 +12,7 @@ import { } from "@query-doctor/core"; import { type Connectable } from "../sync/connectable.ts"; import { DumpCommand, RestoreCommand } from "../sync/schema-link.ts"; +import { describeCommandFailure } from "../sync/command-failure.ts"; import { ConnectionManager } from "../sync/connection-manager.ts"; import { ExtensionNotInstalledError } from "../sync/errors.ts"; import { type OptimizedQuery, type RecentQuery } from "../sql/recent-query.ts"; @@ -326,11 +327,18 @@ export class Remote extends EventEmitter { source: Connectable, ): Promise { const dump = DumpCommand.spawn(source, "native-postgres"); - // is copying up events like this a good idea? + // Keep what these emit, not just forward it. The listeners below reach a + // websocket the live UI subscribes to, and CI has no such subscriber — so + // a failed CI restore reported an exit code while pg_restore had already + // named the extension or collation it could not create. + const dumpOutput: string[] = []; + const restoreOutput: string[] = []; dump.on("dump", (data) => { + dumpOutput.push(data); this.emit("dumpLog", data); }); dump.on("restore", (data) => { + restoreOutput.push(data); this.emit("restoreLog", data); }); @@ -340,12 +348,16 @@ export class Remote extends EventEmitter { ); if (!dumpResult.status.success) { throw new Error( - `Dump failed with status ${dumpResult.status.code}`, + describeCommandFailure("pg_dump", dumpResult.status.code, dumpOutput), ); } if (restoreResult && !restoreResult.status.success) { throw new Error( - `Restore failed with status ${restoreResult.status.code}`, + describeCommandFailure( + "pg_restore", + restoreResult.status.code, + restoreOutput, + ), ); } } diff --git a/src/sync/command-failure.test.ts b/src/sync/command-failure.test.ts new file mode 100644 index 0000000..78b934a --- /dev/null +++ b/src/sync/command-failure.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, test } from "vitest"; +import { describeCommandFailure } from "./command-failure.ts"; + +describe("describeCommandFailure", () => { + test("includes the output the command already produced", () => { + // The failure that cost two CI runs on a real repository. pg_restore said + // exactly why it failed; the analyzer reported only `status 1`, and the + // stderr went to a websocket that CI does not have. + const message = describeCommandFailure("pg_restore", 1, [ + 'pg_restore: error: could not execute query: ERROR: extension "vector" is not available\n', + "pg_restore: warning: errors ignored on restore: 16\n", + ]); + + expect(message).toContain("pg_restore"); + expect(message).toContain("status 1"); + expect(message).toContain('extension "vector" is not available'); + }); + + test("still names the command and code when nothing was captured", () => { + const message = describeCommandFailure("pg_dump", 2, []); + + expect(message).toContain("pg_dump"); + expect(message).toContain("status 2"); + }); + + test("keeps the end of a long output, where the error is", () => { + // pg_restore reports per-object errors and then its summary. Truncating + // from the end would drop the line that says how the run ended. + const noise = Array.from({ length: 500 }, (_, i) => `line ${i}\n`); + const message = describeCommandFailure("pg_restore", 1, [ + ...noise, + "pg_restore: warning: errors ignored on restore: 16\n", + ]); + + expect(message).toContain("errors ignored on restore: 16"); + expect(message).not.toContain("line 0\n"); + expect(message.length).toBeLessThan(4000); + }); + + test("reports a signal when the command did not exit with a code", () => { + const message = describeCommandFailure("pg_restore", null, ["boom\n"]); + + expect(message).toContain("pg_restore"); + expect(message).toContain("boom"); + }); +}); diff --git a/src/sync/command-failure.ts b/src/sync/command-failure.ts new file mode 100644 index 0000000..66eaa77 --- /dev/null +++ b/src/sync/command-failure.ts @@ -0,0 +1,37 @@ +/** + * How much of a failed command's output to carry in the thrown error. + * + * pg_restore reports one line per object it could not create, so a schema with + * a missing extension produces hundreds. The cause is in the last of them, and + * in the summary line that follows. + */ +const MAX_OUTPUT_CHARS = 2000; + +/** + * Describes a failed child process using the output it already produced. + * + * The output reaches us as events and was previously forwarded to a websocket, + * which the live UI subscribes to and CI does not. A CI job therefore saw + * `Restore failed with status 1` and nothing else, while pg_restore had already + * printed the missing extension or collation by name. Diagnosing it meant + * reproducing the restore by hand outside CI. + */ +export function describeCommandFailure( + command: string, + code: number | null, + output: readonly string[], +): string { + const status = code === null ? "no exit code" : `status ${code}`; + const captured = tail(output.join("")); + if (!captured) { + return `${command} failed with ${status}, and produced no output.`; + } + return `${command} failed with ${status}:\n${captured}`; +} + +/** The end of the output, where the error and the summary are. */ +function tail(output: string): string { + const trimmed = output.trimEnd(); + if (trimmed.length <= MAX_OUTPUT_CHARS) return trimmed; + return `[earlier output omitted]\n${trimmed.slice(-MAX_OUTPUT_CHARS)}`; +}