Skip to content

feat(clients/typescript): bring driver to parity with Go (API surface, tests, packaging)#83

Merged
NikolayS merged 7 commits intomainfrom
chore/typescript-driver-parity-and-tests
Apr 30, 2026
Merged

feat(clients/typescript): bring driver to parity with Go (API surface, tests, packaging)#83
NikolayS merged 7 commits intomainfrom
chore/typescript-driver-parity-and-tests

Conversation

@NikolayS
Copy link
Copy Markdown
Owner

Rewrites clients/typescript/ from a small example into a publishable, feature-complete driver matching the Go client's API surface and behavior.

Parity table (after this PR)

Capability Go TS before TS after
Connect (factory, eager probe) Connect(ctx, dsn) ✗ constructor only connect(dsn)
Close
Send returns event id int64 ✗ returned void bigint
Default event type "default" ✗ used "message"
Receive
Ack
Nack ✗ missing ✓ + NackOptions
subscribe / unsubscribe indirect
High-level Consumer (handle + start) ✗ missing
Cancellation context.Context AbortSignal
Poll interval option WithPollInterval ConsumerOptions.pollInterval
bigint for msg_id / batch_id int64 number (precision loss) ✓ JS bigint
Error class hierarchy (idiomatic Go errors) ✗ generic Error PgqueError + 4 subclasses
Pool (concurrent producers) ✓ pgxpool ✗ single Client ✓ pg.Pool

Bugs found & fixed in the prior driver

Severity Issue
HIGH Message.msg_id/batch_id/retry_count typed as numberpg returns Postgres bigint as string by default; using number silently corrupts data above Number.MAX_SAFE_INTEGER. Now JS bigint.
HIGH send() returned void, dropping the SQL function's bigint event id. Now returns bigint.
HIGH No Nack, no high-level Consumer — half the surface the Go driver exposes.
HIGH package.json was private: true / name pgque-ts-example. Couldn't ship to npm. Now pgque@0.2.0.
HIGH No LICENSE in module root (npm requires it; pkg.go.dev-equivalent). Added Apache-2.0.
HIGH No tests. Added env-gated vitest suite.
MEDIUM Default event type was "message" while SQL pgque.send defaults to "default". Semantic drift. Fixed.
MEDIUM Single Client (single connection) — no pool. Concurrent producers serialized + reconnect-fragile. Now pg.Pool.
LOW Hardcoded DSN in src/smoke.ts. File deleted.
LOW No error class hierarchy / no input validation. Added.

Tests (env-gated via PGQUE_TEST_DSN)

test/client.test.ts, test/consumer.test.ts, test/nack.test.ts — 22 tests total. Without PGQUE_TEST_DSN only the bad-DSN test runs (1 passes); others auto-skip cleanly. With a live DB the suite covers:

  • happy path send → forceTick → receive → ack
  • default event type → "default"
  • empty receive returns []
  • input validation (empty queue/consumer, non-positive max, non-bigint batchId)
  • error mapping: send to nonexistent queue → PgqueQueueNotFoundError
  • payload edge cases: empty obj/arr, null, unicode, deep nesting, number boundary
  • bigint precision preserved across receive/ack
  • subscribe idempotency, unsubscribe removes
  • 50 concurrent producers via Promise.all (verifies pool)
  • nack routing: first nack → retry_queue, not DLQ
  • nack with custom retryAfter / reason
  • nack at retry limit → dead_letter
  • Consumer dispatches by event type (3 mixed messages, 2 handlers)
  • Handler error → only that message nacked, batch still acked
  • AbortSignal stops consumer.start() promptly even with long pollInterval
  • Unhandled message types are nacked (not silently consumed)

How to run locally

cd clients/typescript
npm install
npm run check    # tsc strict on src + test
npm run build    # emits dist/
PGQUE_TEST_DSN=postgres://postgres:pgque_test@localhost/pgque_test npm test

Verified in this branch

  • npm install clean
  • npm run check passes (both tsconfig.json and tsconfig.test.json)
  • npm run build clean
  • npx vitest --run: 1 passed, 21 skipped (no DSN locally)

Out of scope / deferred

  • Publishing to npm registry (npm publish) — release-time decision.
  • TypeScript-side outbox / transactional-enqueue helper — deferred to a follow-up.
  • Observability hooks (OTel / Prometheus) — deferred to v0.3.
  • Listen/Notify push consumer — deferred (poll loop is sufficient for v0.2).

Bug spotted in Go driver (not fixed here)

While auditing the Nack composite-type call shape, I count 12 placeholders in the Go SQL string ROW(\$2..\$12)::pgque.message against 11 supplied args — and the pgque.message composite type itself has 10 fields, so even ROW(\$2..\$11) would be more correct. Either Go has a real bug here or I'm miscounting. Worth a manual eyeball at clients/go/pgque.go:97. The TS implementation in this PR uses 10 ROW elements + \$1/\$12/\$13 for batch/retryAfter/reason and is verified against the SQL signature.

Anti-leak

No mention of any private context anywhere in the diff or commit message.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

…, tests, packaging)

- Rewrite TS client around pg.Pool with feature parity to Go driver: connect, send,
  receive, ack, nack, subscribe, unsubscribe, newConsumer with handle/start.
- Use JS bigint for msg_id, batch_id, and send() return — silent number-precision
  loss above 2^53 was a real correctness bug. Promote pg-types parser at module
  load time; documented as a side effect in README.
- Add Consumer with AbortSignal-based cancellation, per-event-type dispatch,
  per-message nack on handler failure or unknown event type.
- Error class hierarchy: PgqueError + Connection / QueueNotFound / ConsumerNotFound
  / Sql subclasses, mapped from raw pg errors.
- Comprehensive vitest suite, env-gated via PGQUE_TEST_DSN: round-trip,
  default-type, validation, error mapping, payload edge cases, bigint preservation,
  subscribe idempotency, concurrent producers, nack routing to retry vs DLQ at
  the limit, custom retryAfter/reason, consumer dispatch + cancellation.
- Publishable package: name 'pgque' (not 'pgque-ts-example'), version 0.2.0,
  Apache-2.0 LICENSE in module root, ESM exports map, engines >=20.
- TSDoc on every exported symbol; strict + noUncheckedIndexedAccess.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@NikolayS
Copy link
Copy Markdown
Owner Author

REV Review

CI: ⚠️ client-smoke failing. claude-review passed (8m13s); test (14/15/16/17/18), verify all green.

Functional evidence: npm install clean per CI logs; npm run check (both tsconfigs strict + noUncheckedIndexedAccess) clean; vitest skips correctly without PGQUE_TEST_DSN. Live-DB test pass count: 22 (15 client + 4 consumer + 3 nack), all env-gated.

Verdict: FIXES NEEDED

Blocking

  • [HIGH] [10/10] .github/workflows/ci.ymlclient-smoke step still runs npx tsx src/smoke.ts, but this PR deletes clients/typescript/src/smoke.ts. Failure log: ERR_MODULE_NOT_FOUND: Cannot find module '.../clients/typescript/src/smoke.ts'. Fix one of:
    • (a) Restore an env-gated clients/typescript/src/smoke.ts that exits 0 if PGQUE_TEST_DSN is unset, runs a minimal end-to-end flow otherwise. Cheapest path.
    • (b) Replace the workflow line npx tsx src/smoke.ts with npx vitest --run (or a single test run). Pulls all 22 cases into CI; nicer long-term but more workflow churn.
    • Recommend (a) for v0.2.0, switch to (b) in v0.2.1 once the test infra settles.

Non-blocking

  • [MEDIUM] [8/10] src/client.ts:23 types.setTypeParser(20, ...) mutates a process-global parser table — every other pg consumer in the same process now sees BIGINT columns as JS bigint instead of strings. Code comment documents this, but README.md should also call it out under a "Caveats" section so a user dropping pgque into an existing app isn't surprised.
  • [LOW] [7/10] Consumer.start() AbortSignal interrupts only the sleep() between polls — an in-flight client.receive() finishes its round-trip before the loop exits. Worth one TSDoc line on start(). Acceptable for v0.2.0.

Potential

  • [6/10] pg.Pool default size is 10, but client.test.ts 50 concurrent producers test will serialize 5-deep. Test will still pass if assertions are about unique IDs / completion; would be a problem only if elapsed-time bounds are asserted. Verify in a live run.
  • [6/10] JSON.stringify(event.payload) on send() — throws on undefined or circular references. Wrapped error path is fine, but a one-line input-shape comment in TSDoc would save users a debug session.
  • [6/10] Message.payload: string — removed implicit JSON.parse() is the right call (consumer chooses impl), but README quickstart shows JSON.parse(msg.payload) in the consumer body. Worth a short note in README that payload comes back as raw JSON text.

Anti-leak: ✅ clean. Zero hits for gitlab|sahmed|artifact[_-]?registry|@AR\b across all .ts, README, package.json.


REV review (security, bugs, tests, guidelines, docs). SOC2 items skipped per project policy. claude-review GitHub Action passed on this PR (auth-403 from earlier runs not reproducing here).

NikolayS and others added 6 commits April 29, 2026 20:40
Temporary bridge until PR #84 lands and replaces
`client-smoke` CI step with `client-tests` (vitest).
Exits 0 if PGQUE_TEST_DSN is unset; runs minimal
send→tick→receive→ack otherwise.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents that importing pgque mutates the process-global
pg-types parser (oid 20 → JS bigint) and what that means
for other pg-using code in the same process.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Clarifies that aborting the signal cuts the inter-poll sleep
immediately but does not cancel an in-flight receive() call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Default pool size 10 is intentional: the test asserts uniqueness
and count only, no elapsed-time bounds, so 5-deep queueing is
acceptable. Comment added inline to the test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents that undefined/circular refs cause JSON.stringify to
throw or silently drop values, and that only plain JSON-compatible
values are safe.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
payload is raw JSON text; consumer must call JSON.parse().
Added inline note and updated the handler example.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
Drop noisy npm ci-then-fallback pattern in favor of an explicit lockfile
check, and add --passWithNoTests to vitest so the job stays green on
branches that don't yet ship tests under clients/typescript/. Once PR #83
lands its vitest suite, the same command runs the new specs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@NikolayS
Copy link
Copy Markdown
Owner Author

REV Review — PR #83 (round 2)

CI: All green — claude-review 4m52s, client-smoke 56s, test (14/15/16/17/18) + verify all pass.

Round 1 findings status:

  • B1 (smoke.ts CI break) — addressed correctly. New clients/typescript/src/smoke.ts (commit 659af2e) is env-gated: exits 0 if PGQUE_TEST_DSN unset, runs connect → create_queue → subscribe → send → forceTick → receive → ack round-trip otherwise. No DSN hardcoded. Disposable queue name smoke_ts_${Date.now()} + cleanup in finally. Header comment notes it as a temporary bridge until PR ci: client-tests job runs all 3 driver suites against live PG #84.
  • NB1 (global BIGINT parser caveat in README) — addressed correctly. Commit 8dc934a adds a "Caveats" section with practical impact bullets, rationale (matches Go/Python drivers), and "no opt-out" note. Reads clearly + factually.
  • NB2 (Consumer.start AbortSignal scope) — addressed correctly. Commit 0203baf adds an "Abort granularity" TSDoc paragraph on Consumer.start() explaining that abort cuts the inter-poll sleep but does not cancel an in-flight client.receive().
  • P1 (50-producer pool-size justification) — addressed correctly. Commit f8adf15 adds an inline test comment explaining default pool size 10 → 5-deep queueing → test asserts uniqueness/count only, no time bounds.
  • P2 (send TSDoc on payload edge cases) — addressed correctly. Commit 93ebb03 adds "Payload shape requirements" block to send() covering undefined/circular/BigInt/top-level-undefined behavior.
  • P3 (README JSON.parse note) — addressed correctly. Commit 1eb0f49 updates Quickstart handler to const data = JSON.parse(msg.payload) as { id: number } plus a one-line note that msg.payload is raw JSON text.

Functional evidence: Read all six round-2 commits and the resulting tip-of-branch state of src/smoke.ts, src/client.ts, src/consumer.ts, clients/typescript/README.md, test/client.test.ts. Verified the pgque.subscribe/unsubscribe SQL functions actually exist in sql/pgque.sql. Confirmed strict TS + noUncheckedIndexedAccess in both tsconfigs. Inspected the CI logs for client-smoke job (run 25146144639).

Verdict: READY FOR USER REVIEW (with one CI-signal note worth flagging for follow-up)

Blocking

None.

Non-blocking

  • [MEDIUM] [7/10] CI smoke step is now a silent no-op. The TypeScript smoke step in .github/workflows/ci.yml:123-128 does not export PGQUE_TEST_DSN, so src/smoke.ts exits 0 immediately without running the round-trip. CI logs confirm: only npm install + npm run check output appears, no pgque TypeScript smoke test: OK line. The Python and Go smoke steps in the same job DO connect (they use the docker PG started above). The PR resolves B1 (CI no longer crashes) but at the cost of removing the actual smoke signal that existed before this PR — the prior src/smoke.ts on main hardcoded the test DSN and ran a full round-trip. The commit message acknowledges this is a temporary bridge until PR ci: client-tests job runs all 3 driver suites against live PG #84 lands; tracking that follow-up is fine. Quick fix in this PR (one-liner) would be PGQUE_TEST_DSN=postgres://postgres:pgque_test@localhost/pgque_test npx tsx src/smoke.ts in the workflow step. Not blocking because Python+Go smoke still cover the schema, and npm run check does cover TS compile correctness.

  • [LOW] [6/10] Root README.md TS section is stale. The repo-root README.md still shows new PgqueClient(...) and client.connect() — the pre-rewrite API. After this PR, the public quickstart is const client = await connect(dsn). Out of strict scope for this PR (root README), but worth a follow-up commit.

Potential

  • [6/10] concurrent producers test ack pattern. test/client.test.ts:188-205 calls env.client.ack(batch[0]!.batchId) once per receive. Across multiple batches that loop will work because each batch has its own batch_id, and total < N will eventually break. Confirmed correct, just non-obvious — leave as-is.
  • [6/10] Commit subject length. Per CLAUDE.md "Subject < 50 chars": round-2 commit subjects are 57–62 chars (docs(ts): TSDoc on Consumer.start() re AbortSignal scope (NB2) = 62), and the feat commit subject from round-1 is 88 chars. Worth a quiet reminder; no rebase needed for v0.2.0.
  • [5/10] Message.retryCount: number | null. pg returns bigint (oid 20) as JS bigint after setTypeParser(20, ...). pgque.message.retry_count is int4, not bigint, so number is correct — verified in SQL composite definition. Consider if any future change to int8 would silently break the type. Non-issue today; leave as-is.

Summary table

Severity Count
Blocking 0
Non-blocking 2
Potential 3

Anti-leak

Clean. grep -iE "gitlab\|sahmed\|artifact[_-]?registry\|@AR\b\|wi[ -]?#?7[67]\|round[ -]?8\|R8\b\|prospect" returns zero hits across the diff, PR body, and all six round-2 commit messages.


REV-style review (security, bugs, tests, guidelines, docs). SOC2 items skipped per project policy. All round-1 findings addressed; surfacing one new CI-signal regression (smoke step is now a no-op) as MEDIUM non-blocking with a one-liner fix path.

@NikolayS NikolayS merged commit c392307 into main Apr 30, 2026
8 checks passed
@NikolayS NikolayS deleted the chore/typescript-driver-parity-and-tests branch April 30, 2026 09:34
NikolayS added a commit that referenced this pull request Apr 30, 2026
client-smoke ran only TestSmoke-named tests and didn't set
PGQUE_TEST_DSN, so env-gated integration tests silently skipped.
That's how the Nack placeholder bug shipped green in PR #79.

New client-tests job:
- Sets PGQUE_TEST_DSN pointing at the docker PG.
- Runs full Go suite: go test -race -v ./...
- Runs full Python suite: pytest -v clients/python/tests/
- Runs full TypeScript suite: npx vitest --run (replaces tsx src/smoke.ts).

Switching the TS step to vitest also unblocks PR #83, which deletes
the hardcoded src/smoke.ts in favor of the vitest suite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
Drop noisy npm ci-then-fallback pattern in favor of an explicit lockfile
check, and add --passWithNoTests to vitest so the job stays green on
branches that don't yet ship tests under clients/typescript/. Once PR #83
lands its vitest suite, the same command runs the new specs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
PR #82 (Python) and #83 (TypeScript) are now merged on main.
Drop --ignore flags for test_consumer.py/test_smoke.py (Python)
and --passWithNoTests (TypeScript); all real tests now run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
client-smoke ran only TestSmoke-named tests and didn't set
PGQUE_TEST_DSN, so env-gated integration tests silently skipped.
That's how the Nack placeholder bug shipped green in PR #79.

New client-tests job:
- Sets PGQUE_TEST_DSN pointing at the docker PG.
- Runs full Go suite: go test -race -v ./...
- Runs full Python suite: pytest -v clients/python/tests/
- Runs full TypeScript suite: npx vitest --run (replaces tsx src/smoke.ts).

Switching the TS step to vitest also unblocks PR #83, which deletes
the hardcoded src/smoke.ts in favor of the vitest suite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
Drop noisy npm ci-then-fallback pattern in favor of an explicit lockfile
check, and add --passWithNoTests to vitest so the job stays green on
branches that don't yet ship tests under clients/typescript/. Once PR #83
lands its vitest suite, the same command runs the new specs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
PR #82 (Python) and #83 (TypeScript) are now merged on main.
Drop --ignore flags for test_consumer.py/test_smoke.py (Python)
and --passWithNoTests (TypeScript); all real tests now run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
client-smoke ran only TestSmoke-named tests and didn't set
PGQUE_TEST_DSN, so env-gated integration tests silently skipped.
That's how the Nack placeholder bug shipped green in PR #79.

New client-tests job:
- Sets PGQUE_TEST_DSN pointing at the docker PG.
- Runs full Go suite: go test -race -v ./...
- Runs full Python suite: pytest -v clients/python/tests/
- Runs full TypeScript suite: npx vitest --run (replaces tsx src/smoke.ts).

Switching the TS step to vitest also unblocks PR #83, which deletes
the hardcoded src/smoke.ts in favor of the vitest suite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
Drop noisy npm ci-then-fallback pattern in favor of an explicit lockfile
check, and add --passWithNoTests to vitest so the job stays green on
branches that don't yet ship tests under clients/typescript/. Once PR #83
lands its vitest suite, the same command runs the new specs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
PR #82 (Python) and #83 (TypeScript) are now merged on main.
Drop --ignore flags for test_consumer.py/test_smoke.py (Python)
and --passWithNoTests (TypeScript); all real tests now run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
client-smoke ran only TestSmoke-named tests and didn't set
PGQUE_TEST_DSN, so env-gated integration tests silently skipped.
That's how the Nack placeholder bug shipped green in PR #79.

New client-tests job:
- Sets PGQUE_TEST_DSN pointing at the docker PG.
- Runs full Go suite: go test -race -v ./...
- Runs full Python suite: pytest -v clients/python/tests/
- Runs full TypeScript suite: npx vitest --run (replaces tsx src/smoke.ts).

Switching the TS step to vitest also unblocks PR #83, which deletes
the hardcoded src/smoke.ts in favor of the vitest suite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
Drop noisy npm ci-then-fallback pattern in favor of an explicit lockfile
check, and add --passWithNoTests to vitest so the job stays green on
branches that don't yet ship tests under clients/typescript/. Once PR #83
lands its vitest suite, the same command runs the new specs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
PR #82 (Python) and #83 (TypeScript) are now merged on main.
Drop --ignore flags for test_consumer.py/test_smoke.py (Python)
and --passWithNoTests (TypeScript); all real tests now run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant