test(nextly): fail integration tests that leave a transaction aborted

.changeset/pg-aborted-transaction-guard.md

@@ -0,0 +1,27 @@
+---
+"nextly": patch
+"create-nextly-app": patch
+"@nextlyhq/admin": patch
+"@nextlyhq/admin-css": patch
+"@nextlyhq/blocks-engine": patch
+"@nextlyhq/ui": patch
+"@nextlyhq/adapter-drizzle": patch
+"@nextlyhq/adapter-postgres": patch
+"@nextlyhq/adapter-mysql": patch
+"@nextlyhq/adapter-sqlite": patch
+"@nextlyhq/storage-s3": patch
+"@nextlyhq/storage-uploadthing": patch
+"@nextlyhq/storage-vercel-blob": patch
+"@nextlyhq/plugin-form-builder": patch
+"@nextlyhq/plugin-page-builder": patch
+"@nextlyhq/plugin-seo": patch
+"@nextlyhq/plugin-sdk": patch
+"@nextlyhq/eslint-config": patch
+"@nextlyhq/prettier-config": patch
+"@nextlyhq/telemetry": patch
+"@nextlyhq/tsconfig": patch
+---
+
+Integration tests now fail when a PostgreSQL transaction is left aborted, which catches a class of bug that previously only showed up in production. Checking whether something exists by running a query and catching the failure works on SQLite and MySQL, but on PostgreSQL the failed query poisons the whole transaction, so every statement after it fails too. The suite could be green on two databases and quietly broken on the third.
+
+Transactions opened through `createTestNextly` (`nextly/testing`) are now checked once after the callback returns, so an abort is still detected when the callback catches the failure itself and returns normally. That case leaves PostgreSQL accepting the commit as a rollback, so the write silently keeps nothing. Test-only: nothing on the request path changes.

packages/nextly/src/__tests__/aborted-transaction-sightings.test.ts

@@ -0,0 +1,117 @@
+/**
+ * Detection has to be structural, because the message is not stable.
+ *
+ * PostgreSQL translates its human-readable error text according to the server's `lc_messages`, so
+ * an English substring match fails to fire on a localized server. That failure is silent and it
+ * fails open: nothing is recorded, the shared assertion sees an empty buffer, and the suite
+ * reports green over a transaction that discarded its writes. SQLSTATE `25P02` is fixed by the
+ * wire protocol and carries the same meaning on every server.
+ *
+ * No database needed — these are the shapes the adapter and the driver actually produce, asserted
+ * directly.
+ */
+import { describe, expect, it } from "vitest";
+
+import {
+  PG_ABORTED_TRANSACTION_SQLSTATE,
+  isAbortedTransactionError,
+  recordAbortedTransaction,
+  takeAbortedTransactionSightings,
+} from "./aborted-transaction-sightings";
+
+/** The `DatabaseError` shape `createDatabaseError` produces, minus the fields not read here. */
+function classified(fields: {
+  message: string;
+  code?: string;
+  cause?: unknown;
+}): Error & { code?: string } {
+  const error = new Error(fields.message) as Error & {
+    code?: string;
+    cause?: unknown;
+  };
+  if (fields.code !== undefined) error.code = fields.code;
+  if (fields.cause !== undefined) error.cause = fields.cause;
+  return error;
+}
+
+describe("isAbortedTransactionError", () => {
+  it("matches on SQLSTATE when the message is not English", () => {
+    // A German `lc_messages` server reporting 25P02. The English matcher cannot see this, which
+    // is the whole reason the code is consulted first.
+    const error = classified({
+      message:
+        "FEHLER: aktuelle Transaktion wurde abgebrochen, Befehle werden bis zum Ende des Transaktionsblocks ignoriert",
+      code: PG_ABORTED_TRANSACTION_SQLSTATE,
+    });
+
+    expect(isAbortedTransactionError(error)).toBe(true);
+  });
+
+  it("matches on the message when the code was stripped on the way up", () => {
+    const error = classified({
+      message:
+        "current transaction is aborted, commands ignored until end of transaction block",
+    });
+
+    expect(isAbortedTransactionError(error)).toBe(true);
+  });
+
+  it("finds the code on the cause when the wrapper does not carry it", () => {
+    // What the adapter produces: its own classified error wrapping the driver's, where only the
+    // inner one carries the SQLSTATE.
+    const error = classified({
+      message: "Query failed",
+      cause: classified({
+        message: "aktuelle Transaktion wurde abgebrochen",
+        code: PG_ABORTED_TRANSACTION_SQLSTATE,
+      }),
+    });
+
+    expect(isAbortedTransactionError(error)).toBe(true);
+  });
+
+  it("matches a bare string carrying the driver text", () => {
+    expect(
+      isAbortedTransactionError(
+        "current transaction is aborted, commands ignored"
+      )
+    ).toBe(true);
+  });
+
+  it("ignores unrelated database errors", () => {
+    const unique = classified({
+      message:
+        'duplicate key value violates unique constraint "posts_slug_key"',
+      code: "23505",
+    });
+
+    expect(isAbortedTransactionError(unique)).toBe(false);
+    expect(isAbortedTransactionError(new Error("connection terminated"))).toBe(
+      false
+    );
+    expect(isAbortedTransactionError(undefined)).toBe(false);
+    expect(isAbortedTransactionError(null)).toBe(false);
+    expect(isAbortedTransactionError({})).toBe(false);
+  });
+
+  it("terminates on a self-referencing cause chain", () => {
+    // A wrapper that points at itself would spin a naive walk forever, taking the suite with it.
+    const looping = classified({ message: "wrapped" });
+    (looping as { cause?: unknown }).cause = looping;
+
+    expect(isAbortedTransactionError(looping)).toBe(false);
+  });
+});
+
+describe("the sightings buffer", () => {
+  it("hands back what was recorded and clears itself", () => {
+    // Held on globalThis so the source module and the copy bundled into `dist/testing.mjs` share
+    // one array; a per-module array would let a recorded abort go unseen by the assertion.
+    recordAbortedTransaction("first");
+    recordAbortedTransaction("second");
+
+    expect(takeAbortedTransactionSightings()).toEqual(["first", "second"]);
+    // Cleared on read, so one test's failure is never re-reported against the next.
+    expect(takeAbortedTransactionSightings()).toEqual([]);
+  });
+});

packages/nextly/src/__tests__/aborted-transaction-sightings.ts

@@ -0,0 +1,96 @@
+/**
+ * Shared buffer and detection for PostgreSQL aborted-transaction sightings.
+ *
+ * Deliberately dependency-free. `src/__tests__/setup.ts` is the setup file for BOTH the unit and
+ * the integration vitest configs, so anything it imports is loaded into every unit test too.
+ * Importing the integration harness there pulls in the DI registry, the adapters and the event
+ * bus, which is enough to break unit suites that expect none of it. Keeping the buffer in its own
+ * module lets the assertion live in the shared setup without dragging the harness along.
+ *
+ * @module __tests__/aborted-transaction-sightings
+ */
+
+/**
+ * PostgreSQL's SQLSTATE for "an earlier statement in this transaction failed".
+ *
+ * The authoritative signal. Unlike the message text this is fixed by the wire protocol, so it
+ * survives a server running with a non-English `lc_messages` — where the human-readable text is
+ * translated and an English substring match would silently never fire, leaving the suite falsely
+ * green.
+ */
+export const PG_ABORTED_TRANSACTION_SQLSTATE = "25P02";
+
+/**
+ * The English text for the same condition.
+ *
+ * Kept as a fallback for errors that reach us with the code stripped: the adapter preserves
+ * `code` on the errors it classifies, but a value re-wrapped further up the stack may carry only
+ * a message. Matching on both means the weaker signal never has to stand alone.
+ */
+export const PG_ABORTED_TRANSACTION = "current transaction is aborted";
+
+/** How deep to follow `cause` before giving up, so a self-referencing chain cannot spin. */
+const MAX_CAUSE_DEPTH = 10;
+
+/**
+ * Whether a thrown value reports an aborted transaction.
+ *
+ * Walks the `cause` chain because the PostgreSQL adapter classifies the driver's error into a
+ * `DatabaseError` and keeps the original underneath: the code can be on either one, and which
+ * depends on where in the stack the value was caught.
+ */
+export function isAbortedTransactionError(error: unknown): boolean {
+  let current: unknown = error;
+  for (let depth = 0; depth < MAX_CAUSE_DEPTH && current != null; depth += 1) {
+    if (typeof current === "string") {
+      if (current.includes(PG_ABORTED_TRANSACTION)) return true;
+      return false;
+    }
+    if (typeof current !== "object") return false;
+
+    const candidate = current as { code?: unknown; message?: unknown };
+    if (candidate.code === PG_ABORTED_TRANSACTION_SQLSTATE) return true;
+    if (
+      typeof candidate.message === "string" &&
+      candidate.message.includes(PG_ABORTED_TRANSACTION)
+    ) {
+      return true;
+    }
+    current = (current as { cause?: unknown }).cause;
+  }
+  return false;
+}
+
+/**
+ * The buffer, on `globalThis` rather than in module scope.
+ *
+ * Module scope would give one array per module instance, and there is more than one instance. A
+ * test importing the harness through the published `nextly/testing` subpath gets the copy bundled
+ * into `dist/testing.mjs`, while `setup.ts` imports this source file: an abort recorded in one
+ * array is invisible to an assertion reading the other, which fails open and reports green. The
+ * same reasoning applies to Turbopack re-executing modules across an HMR cycle, and is why
+ * `init/schema-snapshot-cache.ts` stores its caches the same way.
+ */
+interface SightingsBag {
+  __nextly_abortedTransactionSightings?: string[];
+}
+
+function sightings(): string[] {
+  const bag = globalThis as SightingsBag;
+  bag.__nextly_abortedTransactionSightings ??= [];
+  return bag.__nextly_abortedTransactionSightings;
+}
+
+/** Record an aborted-transaction error. Called by the integration harness. */
+export function recordAbortedTransaction(message: string): void {
+  sightings().push(message);
+}
+
+/**
+ * Everything seen since the last read, clearing as it goes so one test's failure cannot be
+ * re-reported against the next.
+ */
+export function takeAbortedTransactionSightings(): string[] {
+  const buffer = sightings();
+  return buffer.splice(0, buffer.length);
+}

packages/nextly/src/__tests__/setup.ts

@@ -1,10 +1,39 @@
-import { beforeAll, afterAll } from "vitest";
+import { beforeAll, afterAll, afterEach, expect } from "vitest";
+
+import { takeAbortedTransactionSightings } from "./aborted-transaction-sightings";
 
 beforeAll(() => {
   // Setup test environment
   console.log("🧪 Setting up nextly tests...");
 });
 
+/**
+ * Fail any test that leaves a PostgreSQL transaction aborted.
+ *
+ * `current transaction is aborted` is always a SECONDARY error: some earlier statement in the
+ * transaction failed and was swallowed, and every statement after it reports this instead. The
+ * pattern that produces it is an existence check written as "run a query and catch the
+ * failure" — valid on SQLite and MySQL, fatal on PostgreSQL, and therefore invisible to a suite
+ * that passes on the first two.
+ *
+ * Asserted centrally rather than per test, because the failure surfaces far from its cause and
+ * no individual test knows to look for it.
+ *
+ * Runs after EVERY test so the failure is attributed to the test that caused it rather than to
+ * whichever one happens to run last. Reported through `expect.fail` so it arrives as an
+ * assertion failure, which is what a test runner knows how to present.
+ */
+afterEach(() => {
+  const sightings = takeAbortedTransactionSightings();
+  if (sightings.length === 0) return;
+  expect.fail(
+    `A PostgreSQL transaction was aborted during this test, which means an earlier ` +
+      `statement inside it failed and was swallowed. Find the swallowed error — it is the ` +
+      `real defect, and this message is only its shadow. Seen ${sightings.length} time(s):\n` +
+      sightings.map(s => `  - ${s}`).join("\n")
+  );
+});
+
 afterAll(() => {
   // Cleanup
   console.log("✅ nextly tests complete");