[glasstiger]

Summary

Once the session had received a query's terminal response, QWP cancel() and grantCredit() threw QwpProtocolError synchronously instead of returning a promise, even while the consumer was still reading buffered batches. A cancel inside the result loop failed with this misleading protocol error. From an abort listener or timer, the throw bypassed .catch() and could crash a Node.js process. The fix applies to both the Node.js and browser clients.

Changes

• Calls on completed queries now resolve without sending anything, including through queryViews() handles.
• Control-method failures reject their promises instead of throwing synchronously. Invalid credit amounts reject with RangeError.
• No extra credit is sent after a query consumer stops reading.
• Updates API docs and clarifies that restoring an API's promised behavior is a bug fix, not a breaking change.

Validation

Regression tests cover completed queries, view handles, invalid credit, early consumer exit, and closed or failed sessions.

• Passed: 1,352 Vitest tests, including integration tests, and 39 distribution tests.
• Passed: typechecks, ESLint, and formatting checks.
• Not run: real-Chromium tests.

Fixes #76.

[glasstiger]

Review of 876d7a91 (base 0ce79e1d)

Verdict: approve. No blocking issues; one optional doc-wording suggestion.

Critical

None.

Moderate

None.

Minor

• packages/client-core/src/_qwp/egress-session.ts:424-425: the new TSDoc on QwpEgressViewQuery.grantCredit says an ended query "is owed none: the grant resolves without sending anything." That isn't always true. On an ended queryViews() query, grantCredit(-1) on either the returned handle or the callback's control rejects with RangeError (observed at head). The linked QwpEgressQuery.grantCredit doc and the next sentence in QWP.md do state the exception, but an IDE hover shows only this summary. Suggested wording: "…resolves without sending anything; an invalid additionalBytes still rejects with a RangeError."

How the fix was verified

• The QWP egress: cancel() and grantCredit() throw synchronously once a query has ended #76 crash, reproduced as a standalone Node process. The probe completes a query, then calls void query.cancel().catch(() => undefined) from an AbortSignal listener fired from a timer.
  • At base the process exits with code 1: uncaughtException: QwpProtocolError: QWP response references inactive request ID 0.
  • At head it keeps running and sends no frame.
• Regression tests: with egress-session.ts reverted to base, all 6 new test cases fail and the other 91 pass; at head all pass.
• Static gates pass: pnpm typecheck, typecheck:qwp-browser, typecheck:test, pnpm eslint, and Prettier on the changed .ts files.
• Test suites:
  • vitest run test/qwp test/docs-reference.test.ts: 26 files, 1,084 tests, all pass.
  • The other unit suites: 260 pass. The one failure is the ILP interop test, which needs the questdb-client-test submodule; it wasn't checked out in my scratch worktree, and the test fails the same way at base.
• Mutation checks on the changed guards and try/catch blocks: the new tests catch every mutant except one that only delays when a no-op session.grantCredit() for an ended request settles. That one isn't material, and no change is requested. The returnCredit try/catch this PR removes was dead code.
• Not run: real-Chromium tests. The change uses no Node- or browser-specific APIs.

Behavior changes and tradeoffs (intentional, not defects)

• Errors now arrive as rejections. An invalid credit amount, or a control call on a failed or closed session, now rejects instead of throwing synchronously. Code that wrapped an un-awaited call in try/catch will now see an unhandled rejection. This is intended and documented.
• Numeric strings are refused. At base, query.grantCredit("64") on a running query sent a 64-byte credit; at head it rejects with RangeError (both observed). The session-level QwpEgressSession.grantCredit() doesn't gain that check; it still validates only through the encoder, as before.
• Unknown request IDs are ignored. The session-level cancel, abandon, grantCredit and rejectView now resolve without doing anything for any request ID that isn't the running query, including IDs the session never issued. At base they failed with a misleading QwpProtocolError: cancel, abandon and grantCredit threw synchronously, and rejectView rejected. This matches the new control contract.
• Release classification depends on a policy change in this same PR. Under the base CONTRIBUTING.md rule, these behavior changes would count as breaking. The PR replaces that rule, and the PR follows the new one. Maintainers may want to confirm that a new policy can cover the PR that introduces it.

Summary

• Severity: Critical 0 · Moderate 0 · Minor 1 (in the diff; nothing outside the diff breaks).
• Submodules: none changed.

[glasstiger]

Review of 876d7a91 (base 0ce79e1d)

Verdict: approve. No blocking issues; one optional doc-wording suggestion.

Critical

None.

Moderate

None.

Minor

• packages/client-core/src/_qwp/egress-session.ts:424-425: the new TSDoc on QwpEgressViewQuery.grantCredit says a grant on an ended query "resolves without sending anything". That isn't true for an invalid amount, because validateCredit runs first.
  • At 876d7a91, grantCredit(-1) and grantCredit(1.5) on an ended queryViews() query reject with RangeError. That happens on the handle queryViews() returns and on the control object the callback receives.
  • This sentence is what an editor shows when hovering control.grantCredit in a callback. Only the linked QwpEgressQuery.grantCredit doc mentions the exception.
  • Suggested wording: "…a valid grant resolves without sending anything; an invalid amount still rejects with a RangeError."

Verification

• QWP egress: cancel() and grantCredit() throw synchronously once a query has ended #76 reproduced as a standalone Node process. A query completes, then an AbortSignal listener fired by a timer calls void query.cancel().catch(() => undefined). I ran the same with grantCredit(1).
  • At base the process exits with code 1: QwpProtocolError: QWP response references inactive request ID 0.
  • At head it keeps running and sends no frames.
• The new tests catch the regression.
  • With egress-session.ts reverted to base, all 6 new test cases fail and the other 91 pass.
  • I also removed, one at a time: either terminal-query check, validateCredit, the session-availability check in controlledQuery, and its request-ID comparison. Each change fails at least one new test.
• Credit frames for a retired query that is still draining. Setup: autoCredit: false, the for await loop exits early, then a late grantCredit(64).
  • Head sends QUERY_REQUEST, CANCEL, CREDIT 110, CREDIT 26, then the next query's QUERY_REQUEST.
  • Base sends the same plus CREDIT 64.
  • The query still drains to QUERY_ERROR and the next query runs.
• Gates at head pass: pnpm eslint, typecheck, typecheck:test, typecheck:qwp-browser, Prettier on packages/ and test/, vitest run test/qwp (25 files, 1,082 tests) and test/docs-reference.test.ts.
• Not run: real-Chromium e2e, container integration tests, and test:dist; this PR changes no package boundary.

Coverage

Test gate: pass, no coverage gaps found. Each changed path has a test that fails when its behaviour regresses:

• both query methods, on running, ended, retired, closed and failed queries;
• the view handle and the callback's control object;
• the four session control calls on idle, next-running, closed and failed sessions;
• invalid amounts.

Behaviour changes callers will notice (deliberate)

• Failures now reject. Previously they threw synchronously. A call whose promise is neither awaited nor caught, written to rely on the old throw, now produces an unhandled rejection, and on Node that ends the process. An example is an invalid grantCredit() amount whose promise is ignored inside a queryViews() callback; at base the callback wrapper caught that throw.
• The public session methods cancel, abandon, grantCredit and rejectView now do nothing for any non-active request ID. That includes IDs the session never issued. At base they threw QwpProtocolError.
• QwpEgressQuery.grantCredit("64") now rejects. At base it sent a 64-byte CREDIT. The session-level grantCredit still accepts a numeric string.

Release classification

Under the base CONTRIBUTING.md, these changes would need fix(qwp)!:. This PR's own policy rewrite is what makes them a plain fix:, and it also reclassifies fixes like #78. Merging under the current title means adopting that policy.

Summary

• Severity: Critical 0 · Moderate 0 · Minor 1 (in the diff; nothing outside it breaks).
• Submodules: none changed.