[robobun]

Summary

• Fixed a debug build assertion failure in PostgreSQL error handling when all error message fields are empty
• Added safety check before calling StringBuilder.allocatedSlice() to handle zero-length messages
• Added regression test to prevent future occurrences

The Problem

When PostgreSQL sends an error response with completely empty message fields, the ErrorResponse.toJS function would:

1. Calculate b.cap but end up with b.len = 0 (no actual content)
2. Call b.allocatedSlice()[0..b.len] unconditionally
3. Trigger an assertion in StringBuilder.allocatedSlice() that requires cap > 0

This only affected debug builds since the assertion is compiled out in release builds.

The Fix

Check if b.len > 0 before calling allocatedSlice(). If there's no content, use an empty string instead.

Test Plan

• Added regression test that triggers the exact crash scenario
• Verified test crashes without the fix (debug build)
• Verified test passes with the fix
• Confirmed release builds were not affected

🤖 Generated with Claude Code

[robobun]

^{Updated 5:56 PM PT - Sep 10th, 2025}

❌ @autofix-ci[bot], your commit 902c7b0 has some failures in Build #25683

---

🧪   To try this PR locally:

bunx bun-pr 22558

That installs a local version of the PR into your bun-22558 executable, so you can run:

bun-22558 --bun

[coderabbitai]

Walkthrough

Adds a safeguard in Postgres ErrorResponse.toJS to handle empty error messages without slicing past length, and introduces a regression test that simulates an empty Postgres error packet to ensure no crash under concurrent queries.

Changes

Cohort / File(s)	Summary of changes
Postgres protocol error handling src/sql/postgres/protocol/ErrorResponse.zig	Guard error message construction: pass an empty string when buffer length is zero instead of slicing allocatedSlice()[0..b.len]. Other fields to createPostgresError unchanged.
Regression test for empty error packet test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts	Adds a test that sends a minimal Postgres Error packet from a local TCP server, runs concurrent queries against it, expects errors, and verifies no crash occurs.

Possibly related PRs

• fix(Bun.SQL) fix postgres error handling when pipelining and state reset #22505 — Adjusts Postgres error handling/JS conversion around createPostgresError; touches the same error creation path.

Suggested reviewers

• pfgithub
• cirospaciari

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title concisely and accurately summarizes the primary change: it fixes a PostgreSQL StringBuilder assertion failure caused by empty error messages. It is specific, focused, and appropriate for teammates scanning the project history.
Description Check	✅ Passed	The description provides a clear summary, a problem statement, the implemented fix, and a detailed Test Plan that documents verification steps, which satisfies the template's intent for "what" and "how" even though the exact template headings are not used. All required information to review and verify the change is present and complete.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch claude/fix-postgres-stringbuilder-assertion

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (4)

test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts (4)

6-11: Use socket.end() to avoid racy partial writes

Destroying the socket immediately after write can drop bytes and introduce flakiness. End the socket gracefully so the tiny packet is flushed.

-    socket.write(createPostgresPacket("E", Buffer.from("\0"))); // Terminator only
-    socket.destroy();
+    socket.end(createPostgresPacket("E", Buffer.from("\0"))); // Terminator only, graceful FIN

---

32-38: Assert outcomes instead of a no-op assertion

Capture results and assert every query errored (coerced to null) rather than expect(true).

-  await Promise.all(promises);
-  await sql.close();
-  server.close();
-
-  // If we get here without crashing, the test passes
-  expect(true).toBe(true);
+  const results = await Promise.all(promises);
+  await sql.close();
+  await new Promise<void>(r => server.close(() => r()));
+
+  // Ensure all queries errored (we coerced to null)
+  expect(results.every(r => r === null)).toBe(true);

---

34-34: Wait for server to close before finishing the test

Awaiting close ensures clean teardown and reduces port/socket leaks in CI.

-  server.close();
+  await new Promise<void>(r => server.close(() => r()));

---

1-1: Consider encoding the tracking number in the filename

To align with the repo’s regression test organization (“organize by issue number”), consider renaming to include the PR/issue identifier, e.g., test/regression/issue/22558-postgres-stringbuilder-assertion-aggressive.test.ts.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4b5551d and 902c7b0.

📒 Files selected for processing (2)

• src/sql/postgres/protocol/ErrorResponse.zig (1 hunks)
• test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (8)

test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

• test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts

test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

• test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts

test/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.ts: Name test files *.test.ts and use bun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; use port: 0 to get a random port
When spawning Bun in tests, use bunExe() and bunEnv from harness
Prefer async/await in tests; for a single callback, use Promise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
Use tempDir/tempDirWithFiles from harness for temporary files and directories in tests
For large/repetitive strings in tests, prefer Buffer.alloc(count, fill).toString() over "A".repeat(count)
Import common test utilities from harness (e.g., bunExe, bunEnv, tempDirWithFiles, tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and use toThrow for synchronous errors
Use describe blocks for grouping, describe.each for parameterized tests, snapshots with toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach); track resources for cleanup in afterEach
Use using/await using with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests

Files:

• test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts

test/regression/issue/**

📄 CodeRabbit inference engine (test/CLAUDE.md)

Place regression tests under test/regression/issue/ and organize by issue number

Files:

• test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts

test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: Test files must be placed under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or use custom random port functions
In tests, use normalizeBunSnapshot when asserting snapshots
Never write tests that merely assert absence of "panic" or "uncaught exception" in output
Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Prefer snapshot tests over exact stdout equality assertions

Files:

• test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts

**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Format JavaScript/TypeScript files with Prettier (bun run prettier)

Files:

• test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts

src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Implement debug logs in Zig using const log = bun.Output.scoped(.${SCOPE}, false); and invoking log("...", .{})

Files:

• src/sql/postgres/protocol/ErrorResponse.zig

**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup

Files:

• src/sql/postgres/protocol/ErrorResponse.zig

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Format

🔇 Additional comments (2)

src/sql/postgres/protocol/ErrorResponse.zig (1)

135-137: Fix avoids allocatedSlice() assert when buffer is empty — LGTM

The b.len guard correctly prevents calling allocatedSlice() with cap == 0. This addresses the debug-assert crash without affecting release behavior.

test/regression/issue/postgres-stringbuilder-assertion-aggressive.test.ts (1)

16-20: Confirmed: SQL accepts max and connection_timeout (plus common aliases).
internal/sql/shared.ts normalizes options and reads options.max and options.connectionTimeout / options.connection_timeout / options.connectTimeout / options.connect_timeout (see src/js/internal/sql/shared.ts:465–472). packages/bun-types/sql.d.ts declares connectionTimeout/connection_timeout and max (see packages/bun-types/sql.d.ts:225,304).