Skip to content

Fix PgClient makeClient missing error event handler - #7142

Merged
tim-smart merged 2 commits into
mainfrom
audit/repro-f0011-pgclient-error-handler
Aug 8, 2026
Merged

Fix PgClient makeClient missing error event handler#7142
tim-smart merged 2 commits into
mainfrom
audit/repro-f0011-pgclient-error-handler

Conversation

@fubhy

@fubhy fubhy commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

  • Install a pg.Client error listener before PgClient.makeClient starts connecting, and remove it during scoped release.
  • Replace the one-off audit test with a focused regression in the existing client integration suite.
  • Add a patch changeset for @effect/sql-pg.

Testing

  • EFFECT_INTEGRATION_TESTS=1 pnpm vitest --run packages/sql/pg/test/Client.integration.test.ts -t "PgClient.makeClient handles errors emitted while connecting"
  • pnpm vitest --run --project @effect/sql-pg
  • pnpm --filter @effect/sql-pg check
  • pnpm lint

Closes EFF-570

@fubhy fubhy added bug Something isn't working audit Findings originating from the Effect runtime correctness audit labels Aug 7, 2026
@effect-slopcop effect-slopcop Bot added the 3.0 Used for issues, pull requests, etc. that are relevant for the `v3` branch targeting Effect v3. label Aug 7, 2026
@changeset-bot

changeset-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ad78b45

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@effect/sql-pg Patch
effect Patch
@effect/ai-anthropic Patch
@effect/ai-openai Patch
@effect/ai-openai-compat Patch
@effect/ai-openrouter Patch
@effect/atom-react Patch
@effect/atom-solid Patch
@effect/atom-vue Patch
@effect/docgen Patch
@effect/doctest Patch
@effect/openapi-generator Patch
@effect/opentelemetry Patch
@effect/platform-browser Patch
@effect/platform-bun Patch
@effect/platform-deno Patch
@effect/platform-node Patch
@effect/platform-node-shared Patch
@effect/sql-clickhouse Patch
@effect/sql-d1 Patch
@effect/sql-libsql Patch
@effect/sql-mssql Patch
@effect/sql-mysql2 Patch
@effect/sql-pglite Patch
@effect/sql-sqlite-bun Patch
@effect/sql-sqlite-do Patch
@effect/sql-sqlite-node Patch
@effect/sql-sqlite-react-native Patch
@effect/sql-sqlite-wasm Patch
@effect/vitest Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

This PR currently advertises a fix in its title and changeset, but no source code in packages/sql/pg/src/PgClient.ts is changed. The added test is a placeholder that cannot fail, and pnpm lint fails on it.

Reviewed changes

  • Added a changeset describing a PgClient error-handling fix.
  • Added an integration test for makeClient error-event handling.

⚠️ Fix is described but not implemented

The PR body correctly identifies the bug: makeClient creates a Pg.Client and calls client.connect() without an 'error' listener, and fromClient attaches a no-op function onError() {}. However, the diff only adds a changeset and a test — packages/sql/pg/src/PgClient.ts is untouched. The changeset therefore describes a source fix that does not exist.

⚠️ Placeholder test cannot catch the bug

packages/sql/pg/test/Client.integration.test.ts:466-480 only contains explanatory comments and Effect.void. It does not construct, connect, or observe a Pg.Client, so it will pass regardless of whether the bug is present. A regression test must actually fail before the fix and pass after it.

⚠️ Lint failure

pnpm lint fails with no-unused-expressions on the bare Effect.void statement in the new test.

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | Fix it ➔View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏

@fubhy fubhy added slop 4.0 and removed 3.0 Used for issues, pull requests, etc. that are relevant for the `v3` branch targeting Effect v3. labels Aug 7, 2026
@fubhy
fubhy force-pushed the audit/repro-f0011-pgclient-error-handler branch from 9306ddd to 489359a Compare August 7, 2026 16:52

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Removed the placeholder integration test and the premature changeset.
  • Added a focused regression test in packages/sql/pg/test/Client.test.ts that monkey-patches Pg.Client.prototype.connect to emit an 'error' event during connection and asserts the error is handled.

The test fails on the current base (AssertionError: expected Error: connection failed to equal undefined), confirming it exercises the bug described in the PR body.

ℹ️ PR title scopes the change as a fix

The title says "Fix PgClient makeClient missing error event handler", but the diff and PR body are test-only. The body states this is intentional, so the only suggestion is to align the title with the actual scope (e.g., "Add regression test for PgClient makeClient missing error event handler") so reviewers are not surprised by the absence of a source fix.

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏

@tim-smart
tim-smart enabled auto-merge (squash) August 8, 2026 08:46
@tim-smart
tim-smart merged commit c25b84c into main Aug 8, 2026
18 of 19 checks passed
@tim-smart
tim-smart deleted the audit/repro-f0011-pgclient-error-handler branch August 8, 2026 08:48
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Bundle Size Analysis

Generated from PR build output; treat the content below as untrusted.

File Name Current Size Previous Size Difference
basic.ts 6.92 KB 6.92 KB 0.00 KB (0.00%)
batching.ts 9.72 KB 9.72 KB 0.00 KB (0.00%)
brand.ts 6.60 KB 6.60 KB 0.00 KB (0.00%)
cache.ts 10.59 KB 10.59 KB 0.00 KB (0.00%)
config.ts 20.91 KB 20.91 KB 0.00 KB (0.00%)
differ.ts 19.77 KB 19.77 KB 0.00 KB (0.00%)
http-client.ts 21.52 KB 21.52 KB 0.00 KB (0.00%)
logger.ts 10.81 KB 10.81 KB 0.00 KB (0.00%)
metric.ts 8.86 KB 8.86 KB 0.00 KB (0.00%)
optic.ts 6.68 KB 6.68 KB 0.00 KB (0.00%)
pubsub.ts 14.86 KB 14.86 KB 0.00 KB (0.00%)
queue.ts 11.54 KB 11.54 KB 0.00 KB (0.00%)
schedule.ts 10.71 KB 10.71 KB 0.00 KB (0.00%)
schema-class.ts 19.48 KB 19.48 KB 0.00 KB (0.00%)
schema-fromJsonSchemaDocument.ts 29.36 KB 29.36 KB 0.00 KB (0.00%)
schema-representation-roundtrip.ts 25.63 KB 25.63 KB 0.00 KB (0.00%)
schema-string-transformation.ts 13.55 KB 13.55 KB 0.00 KB (0.00%)
schema-string.ts 11.09 KB 11.09 KB 0.00 KB (0.00%)
schema-template-literal.ts 15.38 KB 15.38 KB 0.00 KB (0.00%)
schema-toArbitrary.ts 21.52 KB 21.52 KB 0.00 KB (0.00%)
schema-toCodeDocument.ts 24.00 KB 24.00 KB 0.00 KB (0.00%)
schema-toCodecJson.ts 18.74 KB 18.74 KB 0.00 KB (0.00%)
schema-toEquivalence.ts 18.57 KB 18.57 KB 0.00 KB (0.00%)
schema-toFormatter.ts 18.43 KB 18.43 KB 0.00 KB (0.00%)
schema-toJsonSchemaDocument.ts 22.59 KB 22.59 KB 0.00 KB (0.00%)
schema-toRepresentation.ts 19.08 KB 19.08 KB 0.00 KB (0.00%)
schema.ts 18.73 KB 18.73 KB 0.00 KB (0.00%)
stm.ts 12.59 KB 12.59 KB 0.00 KB (0.00%)
stream.ts 9.67 KB 9.67 KB 0.00 KB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.0 audit Findings originating from the Effect runtime correctness audit bug Something isn't working slop

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants