Skip to content

feat(postgres-driver): improve DX with connection error messages#10679

Merged
ovr merged 3 commits into
masterfrom
feat/postgres-improve-error
Apr 13, 2026
Merged

feat(postgres-driver): improve DX with connection error messages#10679
ovr merged 3 commits into
masterfrom
feat/postgres-improve-error

Conversation

@ovr

@ovr ovr commented Apr 13, 2026

Copy link
Copy Markdown
Member

Introduce PostgresError/ConnectionError classes and include the pool name in connection failures so users can identify which data source/pool failed. Pool name now encodes dataSource and preAggregations, and driver throws typed errors instead of generic Error.

Introduce PostgresError/ConnectionError classes and include the pool name
in connection failures so users can identify which data source/pool
failed. Pool name now encodes dataSource and preAggregations, and driver
throws typed errors instead of generic Error.
@ovr ovr requested review from a team as code owners April 13, 2026 11:45
@github-actions github-actions Bot added driver:postgres Issues relating to the Postgres driver javascript Pull requests that update Javascript code data source driver labels Apr 13, 2026
@claude

claude Bot commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ovr's task in 2m 12s —— View job


Code Review: feat(postgres-driver): improve DX with connection error messages

  • Read and analyze all changed files
  • Review code quality and error handling
  • Check for security and performance concerns
  • Assess test coverage
  • Post review feedback

Good direction — typed errors and including the pool name in connection failures will meaningfully improve debuggability for multi-datasource setups. A few issues to address before merging:

Blocking

  1. RedshiftDriver.createConnection is broken — The parent createConnection signature changed from (poolConfig) to (poolConfig, poolName), but RedshiftDriver overrides it with the old signature. This will cause a TypeScript compilation error. The override needs to accept and forward poolName. (I couldn't post an inline comment on that file since it's not part of the diff — see details in the inline comments.)

  2. PoolTimeoutError.name regression in pool.ts — The old code set this.name = 'PoolTimeoutError' (the standard Error.name property). The new code removes that and instead sets the poolName field's default to 'PoolTimeoutError', which is semantically wrong (a pool name, not an error name) and gets immediately overwritten. This means error.name will now be 'Error' instead of 'PoolTimeoutError' — a behavioral regression. See inline comment.

Non-blocking

  1. Unsafe e as Error cast in createConnection (PostgresDriver.ts:192) — If pg ever throws a non-Error value, this will produce a misleading error. A defensive e instanceof Error ? e : new Error(String(e)) would be safer. See inline comment.

  2. PostgresError.name should be readonlyConnectionError.name is marked readonly but PostgresError.name is not. Minor inconsistency. See inline comment.

  3. createPoolName placement — This utility function is added to BaseDriver.ts but conceptually belongs with the Pool class in @cubejs-backend/shared. Not a blocker, but worth considering for future refactoring. See inline comment.

Testing

  1. No unit tests for new error classes — There are no tests for PostgresError, ConnectionError, or createPoolName. The existing test suite (PostgresDriver.test.ts) verifies error messages but doesn't check error types (instanceof). Consider adding at least:
    • A test verifying ConnectionError wraps AggregateError correctly (joining messages)
    • A test verifying createPoolName output for both preAggregations: true and false
    • Updating the 'too many params' test to also assert instanceof PostgresError

Summary

The core idea is solid. The main risk is the breaking change to createConnection's signature affecting Redshift (and potentially Materialize/Crate if they override it in the future), and the PoolTimeoutError.name regression. Both should be straightforward fixes.

Comment thread packages/cubejs-backend-shared/src/pool.ts Outdated
Comment thread packages/cubejs-postgres-driver/src/errors.ts
Comment thread packages/cubejs-postgres-driver/src/PostgresDriver.ts
Comment thread packages/cubejs-base-driver/src/BaseDriver.ts
@github-actions github-actions Bot added the driver:redshift Issues relating to the Redshift driver label Apr 13, 2026
@codecov

codecov Bot commented Apr 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.91%. Comparing base (04eed5d) to head (3be522e).
⚠️ Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
packages/cubejs-base-driver/src/BaseDriver.ts 0.00% 4 Missing ⚠️
packages/cubejs-backend-shared/src/pool.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10679      +/-   ##
==========================================
- Coverage   57.92%   57.91%   -0.02%     
==========================================
  Files         215      215              
  Lines       16642    16645       +3     
  Branches     3346     3347       +1     
==========================================
  Hits         9640     9640              
- Misses       6513     6516       +3     
  Partials      489      489              
Flag Coverage Δ
cube-backend 57.91% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ovr ovr merged commit 65fb8a8 into master Apr 13, 2026
78 of 79 checks passed
@ovr ovr deleted the feat/postgres-improve-error branch April 13, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data source driver driver:postgres Issues relating to the Postgres driver driver:redshift Issues relating to the Redshift driver javascript Pull requests that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants