feat(qwp): add browser and Node.js QWP client - #62
Open
glasstiger wants to merge 365 commits into
Open
Conversation
transmit() awaited readFramePayload outside its try block, and enqueueDrain's only handler is an unconditional failTerminal. Any store read failure therefore ended the producer permanently: terminalError is never cleared and QwpSender.getSession caches its session forever, so every later flush rejected, the ingress session closed, no reconnect was attempted, and the journalled frames stayed on disk unreachable in-process. The standalone orphan drainer excludes the foreground's own slot, so only discarding the sender recovered. The trigger is transient by the store's own design. A failed segment trim parks maintenanceFailure, which assertReady raises from readPayload, and a retry clears it about a second later -- a briefly full or read-only filesystem, or a restarted maintenance worker. Commit a42fec5 made the store self-heal precisely so one such failure could not "brick a running store-and-forward producer for the rest of the process lifetime"; the connection layer still did. replayInto makes the identical read and its failures reach connectLoop, where isRetryableReconnectError treats QwpReplayStoreError as retriable and background store-and-forward retries unbounded. Route the drain-path read the same way: mark the frame transmitted so replayInto resends it, keep it off the wire log because nothing reached the wire, and request a reconnect. This is the technique the batch-cap branch directly above already uses. Deterministic corruption -- QwpProtocolError, QwpReplayRejectedError -- still escapes and stays terminal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…read transmit() captured the connection, awaited the journal read, then pushed the frame onto this.wireFrames and sent it on the captured connection. install() replaces wireFrames wholesale and resets wireFramesBase, and replayInto() skips a frame that is not yet transmitted, so a reconnect completing inside that await left the frame written to the dead socket while occupying wire slot n of the replacement -- the slot its next real frame would take. translateResponse clamps an ACK to the wire log and indexes it with no sequence-identity check, so the replacement's first cumulative ACK retired a frame no server had received: acknowledgedFrameSequence advanced past it, its journal record was deleted, and QwpSender had already released the staged rows. No QwpSenderError, no reconnect event, no NACK -- totalFramesSent counted a frame that never reached a server. The window is real in the shipped sf_dir configuration: send() drops frame.payload for a lazy store so every drain reads from disk, and that read queues behind fsyncing appends on the store's shared FIFO -- measured at 5 to 162ms -- while the default reconnect delay is full jitter over [0,100)ms. The producing event is an endpoint that accepts the upgrade and then dies with nothing outstanding: a node restarting, a load balancer recycling sockets. Re-check the connection and the install generation after the read, and retry the frame against the current connection when either moved. The existing coverage only exercised the opposite ordering, where the reconnect starts before requireConnection() returns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
flush(), flushAndGetSequence() and commit() all funnel into enqueueFlushResult, which is not async and calls throwIfUnavailable() as its first statement. A sender that is closed, or closing for the duration of close()'s bounded publish and ACK drain, therefore threw synchronously out of methods whose signature promises a Promise. `sender.flush().catch(handler)` does not catch that, so the common shape -- a periodic flush racing shutdown from a timer or event handler -- turns into an uncaught exception rather than a handled rejection. The sibling members at(), atNow(), connect(), waitForAcknowledged() and close() are all async and reject normally, so the surface was inconsistent with itself. The legacy Sender facade wraps flush() in its own async method, which is why only the qwp entry points were exposed. Declare the three methods async. Their bodies contain no await before the enqueue, so enqueueFlushResult still runs synchronously on the call and flush ordering is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
connectingCandidate is assigned only after the connection factory resolves, so close() issued while a connect was still in flight found nothing to cancel and returned immediately. The socket -- already accepted by the peer but not yet answered at the HTTP upgrade -- and its opening deadline stayed alive until that deadline fired: measured at 15s on the defaults, with getActiveResourcesInfo() still reporting a TCPSocketWrap and a Timeout eight seconds after close() had resolved, and the process exiting exactly when authTimeoutMs elapsed. Give each connect attempt an AbortController, abort it from close(), and have openQwpWebSocket tear the pending socket down on that signal. Both the ingress and egress reconnect loops carry the same defect and are fixed together. The listener is removed once the upgrade settles either way, so a long-lived signal cannot accumulate listeners across reconnects. QwpConnectionFactory gains an optional AbortSignal parameter. A factory that declares no parameters stays assignable, so existing implementations and the webSocketFactory test hooks are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
QwpNodeUdpSession.close() short-circuited on `!this.bound`, and `bound` is set only inside bind()'s success callback. A bind failure -- EMFILE under fd exhaustion, EACCES in a restricted sandbox, EADDRNOTAVAIL -- therefore reached connect()'s cleanup with `bound` still false and skipped socket.close() entirely. node:dgram does not close the handle itself after a bind error (verified: the handle is still present 300ms later), so every failed connect() leaked one descriptor for the process lifetime, and a reconnect loop retrying after EMFILE compounded the exhaustion it was retrying from. Close unconditionally and treat an already-closed socket as done, which is what the Java client's QwpUdpSender.close() does -- it calls channel.close() without consulting bind state. The other constructor-time failure path was already safe: setMulticastTTL/setMulticastInterface throw after `bound` is true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
QWP.md's ingress table showed a dash for auto_flush_rows and auto_flush_interval while giving concrete numbers for every sibling key, so a reader had no way to learn that ws:: applies 1000 rows and 100 ms where http:: applies 75000 and 1000 ms. That is 75x smaller batches and 10x more frequent time-triggered flushes for a workload migrated on the one-line change the migration guide recommends, which the behavioral-differences checklist did not mention either. The values themselves match the Java client, which keeps separate DEFAULT_WS_AUTO_FLUSH_ROWS and DEFAULT_WS_AUTO_FLUSH_INTERVAL constants for exactly this reason, so only the documentation was wrong. Both numbers are now pinned by a test that reads them out of QWP.md and asserts the sender flushes on those thresholds, so the table cannot drift from the code again. Separately, SenderOptions' TSDoc claimed auto_flush_bytes "Defaults to off", but udp -- the only transport that accepts the key -- defaults it to max_datagram_size so datagrams flush before outgrowing the limit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RESOLVED_QWP_SENDER was a module-private Symbol() that nothing ever assigned: its only three references are the declaration, the type that keys on it, and the constructor branch that reads it. Because it is Symbol() rather than Symbol.for() and is not exported, no caller inside or outside the package can produce a SenderOptions carrying that key, so the branch was unreachable. Remove the symbol, the ResolvedQwpSenderOptions type, and the branch. The remaining ws/wss/udp path is unchanged and is the only way a Sender acquires a QwpSender. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
QwpWriterColumn carried its input type on a `unique symbol` key. bunchee emits one self-contained bundle per entry point, so each emitted .d.ts re-declared that symbol, and four nominally distinct keys resulted. The phantom property is optional, so a column built by './qwp' still satisfied './qwp/node''s QwpWriterColumn -- it simply never matched its key, leaving no inference site, so QwpWriterColumnInput fell back to `unknown` and every row field silently accepted anything. That is invisible in this repository: importing from `src/` gives all four entry points one module instance and one symbol, so the in-repo suites and the public API contract typecheck correctly. Only a consumer resolving through package.json `exports` sees the separate declaration files, which is every consumer of the published package -- including the pattern README.md and QWP.md teach. Wrong-typed rows still threw QwpWriterRowError at runtime, so this cost compile-time checking rather than data integrity. Symbol.for() does not help: `declare const x: unique symbol` is nominal per declaration however the value is obtained. Carry the type on a shared property name instead, which resolves structurally across bundles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test:dist loads the built bundles but asserts runtime behaviour only, with `any` throughout its helpers, and no step ran tsc over consumer code at all. The compiled writers promise per-column row typing that lives entirely in the emitted .d.ts files, so nothing in CI could observe it: importing from `src/` gives all four entry points one module instance, which is why `pnpm typecheck` and the public API contract pass even when the published types are inert. Add tsconfig.dist-types(.cjs).json, which resolve @questdb/nodejs-client and its three subpaths to the emitted declarations for both the ESM and CJS emits, over a consumer fixture that exercises a compiled writer built from every entry point. Each check is a `@ts-expect-error`, so the gate fails in both directions: a check that stops firing is reported as an unused directive, which is exactly what a collapse of the row input type looks like. Verified against the previous commit's parent: the four value checks report TS2578 there and pass after it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each entry point emits a self-contained bundle, so a class implemented in src/qwp/** is declared once per bundle. 14 of the 16 classes duplicated between the qwp and qwp/node bundles carry private members, which makes them nominal and their declarations mutually incompatible. qwp/node.d.ts compounds it by re-exporting index's QwpSender wholesale while createQwpNodeSender returns its own local, unexported one, so the importable type and the returned type differ whichever subpath a consumer imports from. Inference is unaffected, which is why nothing caught this: every documented example writes `const sender = await connectQwpNodeSender(...)`. Only explicit annotation breaks -- class fields, parameter and return types -- and no workaround exists, because the correctly-typed declaration is not exported. Record the defect with @ts-expect-error rather than leaving it latent, and pin the two shapes that do work so they cannot regress: inference, and the structural classes with no private members. Collapsing src/qwp/** into one shared chunk gives each class a single declaration, at which point these annotations compile and tsc reports the directives as unused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bunchee bundles each entry point self-contained, so the QWP implementation was inlined into ./qwp, ./qwp/node and ./qwp/browser separately. Anything whose identity depends on its declaration site therefore existed three times, and 14 of the 16 classes duplicated between the bundles carry private members, which makes them nominal. qwp/node.d.ts compounded it: it re-exports index's QwpSender wholesale while createQwpNodeSender returns its own local, unexported one, so `const s: QwpSender = createQwpNodeSender(opts)` failed from every subpath with no workaround, because the correctly-typed declaration was not exported anywhere. The same duplication produced the writer-brand defect fixed earlier; this removes the cause rather than another symptom. Move the implementation under underscore-prefixed paths, which is bunchee's shared-module convention, keeping the four entry files where the exports map expects them. The prefix has to be applied at every level -- _core and _internal as well as _qwp -- because a directory without it is inlined into each shared module that imports it, which leaves the duplication in place. Consumers get smaller graphs, since an entry no longer carries code only its siblings need: ./qwp/browser 942 -> 510 kB, ./qwp/node 1112 -> 682 kB, the root ILP entry 1256 -> 826 kB, ./qwp 452 -> 460 kB, and the published runtime total 1747 -> 845 kB. Two things the layout newly requires. dist/_qwp is outside dist/es and dist/cjs, so `files` must ship it -- npm pack omitted 132 files without that, publishing a package whose every entry imports something missing. And the publish artifact check now follows relative imports out of the exports targets, since no exports entry names a chunk. Browser purity is unchanged and verified through the whole chunk graph: 35 modules, none importing a node: builtin or ws. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The C8 defect existed because the only failover coverage injected a factory throw carrying tryNextEndpoint: true -- a shape a real browser WebSocket cannot produce, since a browser never sees the HTTP response. The unit test added with the fix drives a bare error event through a fake socket, which is closer but still an approximation. Drive real Chromium at a genuinely refused port instead. A probe confirms the browser's own classification is `kind: "opaque"` with retryable and tryNextEndpoint both absent, which is exactly the tri-state the sweep reads, so the endpoint list is walked under the real conditions rather than a modelled one. Verified load-bearing: reverting the failover guard fails this test. The asset server is re-rooted at dist/ because the browser bundle now imports shared chunks from dist/_qwp; serving only dist/es/qwp 403s every entry import, which broke all eleven browser tests until this was fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GitHub disabled this workflow for repository inactivity, which also stopped it firing on pull requests, so the PR gates were silently not running. Manual dispatch is the recovery path that does not require pushing a commit to a branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lock fs-ext-extra-prebuilt is a NAN addon, so it needs a fresh binary for every Node major and ships none past Node 25. Store-and-forward therefore stopped working entirely on Node 26 rather than degrading. Ownership now comes from a `.lock.owner` directory: mkdir is the only exclusive-by-construction filesystem operation available on every supported platform without a native addon. A kernel lock vanished when its holder died, so a heartbeat replaces that: the holder refreshes the directory mtime every 5s and a contender reclaims a slot idle for 15s, or immediately when the recorded PID is gone from the same host. Stale directories are renamed aside before removal so two contenders cannot both win one slot. This drops the guarantee that a Java and a Node client exclude each other on one directory, because Java uses flock/LockFileEx and nothing pure-JS can participate in those. The persistence format stays cross-client for sequential handoff; only concurrent cross-runtime access is now unsupported, and QWP.md states that explicitly. `.lock` and `.lock.pid` are still written so a slot keeps the on-disk shape Java expects. QwpReplayStoreUnavailableError is removed: with no optional native module, nothing can be unavailable. The two tests asserting contention with a real Java flock are removed rather than inverted, since simulating a flock holder required the dependency being dropped; stale-reclaim tests replace them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`auth: {keyId, token}` supplies only the private scalar, so the JWK was
completed with a hardcoded x/y that bears no relation to it. Node accepted
that inconsistent pair without checking up to v24; v26 validates it and
raises ERR_CRYPTO_INVALID_JWK, which breaks ILP TCP authentication outright
for anyone on that runtime.
Derive the point with ECDH instead. Callers passing a complete `jwk` were
never affected, and a derived pair is byte-identical to a correct one, so
authentication behaviour is unchanged on every Node version.
The existing auth tests cannot catch this: they pass with the placeholder on
any Node below v26. The added test compares the point against the private key
directly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three `against QuestDB` cases duplicated server-side coverage that
already exists, and each server-side version is broader:
session cookie auth -> QwpBrowserSessionAuthTest
.testSessionCookieAuthenticatesIngressAndEgress,
plus missing-session rejection, cookie rotation and
the service-account hook, plus the Enterprise
REST/OIDC login suites
durable ACK opt-in -> QwpIngressUpgradeProcessorOnHeadersReadyTest
.testOnHeadersReadyDoesNotSelectBrowserSubprotocol-
WhenRegistryDisabled and its three siblings
version + batch cap -> testBrowserHandshakeAppendsIngressServerInfo,
testOnHeadersReadyAdvertisesEffectiveBatchSize,
QwpEgressMaxBatchRowsTest
Tests needing a live database belong in the repositories that own the
topology and authentication fixtures. What is left here is the eight cases
that drive the built browser bundle in real Chromium against a local mock
server - the client-side half of the same negotiation paths, and the part no
Java suite can cover because it does not run JavaScript.
The job no longer pulls a container: it drops from ~16s to ~2s, stops
depending on questdb/questdb:nightly, and is renamed since it is no longer an
end-to-end suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`pnpm typecheck` included only `src` and one contract file, so nothing type-checked test/** at all. vitest strips types with esbuild rather than checking them, so a test could reference a deleted export and still pass - which is exactly how a dangling import of a removed error class survived every gate in this branch. `tsconfig.test.json` follows the tsconfig.bench.json pattern rather than widening the base config, which bunchee also reads. Turning it on surfaced 22 errors. Two were real: QwpNodeOrphanDrainSession was not exported from src/qwp/node, but the public QwpNodeOrphanDrainerOptions.createSession returns it, so nobody outside this package could implement that interface. Now exported and pinned in the type-position contract. session.test.ts passed `reconnect` in connectQwpBrowserIngress's first argument, where it is not a valid key. It was silently dropped and the test ran on the default backoff rather than the zero backoff it asked for. The rest were test-local: helper parameters whose types were inferred as narrow literals from their default values, header lookups typed `string` where node returns `string | string[]`, a listener returning Array.push's number, node's Blob requiring its sources argument, a duplicated named import, a type argument on toMatchObject, and two `satisfies Partial<T>` that did not account for toMatchObject matching nested objects partially. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test/dist-types imports the package by its published name, which resolves only against a built dist/. tsconfig.test.json included all of test/, so those files were type-checked before any build ran and failed with TS2307. It passed locally only because a dist/ from an earlier build was still present; verified now by removing dist/ first, which reproduces CI. Those files already belong to tsconfig.dist-types*.json, which typecheck:dist runs after pnpm build, so excluding them moves no coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `Check for build artifacts` step could never run. Its body was a single-quoted `node -e '...'` containing a regex character class `['\"]`, and a YAML block scalar performs no escape processing, so three literal single quotes reached the shell. That closes the `node -e` argument at a bare `(`: bash, sh, dash and zsh all fail to parse it, exit 2, and the `Publish` step that follows never runs. Behind that sat a second failure. The walk matched `from "./x"` anywhere in the raw text of an emitted file, including inside a comment the bundler preserved. src/_qwp/writer.ts explains the writer column brand with the sentence "a schema built with the factories from './qwp' would be rejected by the writer() of a sender imported from './qwp/node'", which the pattern read as two imports and reported as four missing artifacts, exiting 1 against a clean build. Move the script to scripts/check-build-artifacts.mjs, where it needs no shell quoting, and anchor the pattern to specifiers that name an emitted file so prose cannot look like an import. Also run it in build.yml: the gate existing only in the release workflow is why neither failure was visible on a pull request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A multi-address Node client closed while it was reconnecting terminated the host process with an uncaught exception and exit code 1. close() aborts the shared connect AbortSignal. The endpoint being negotiated rejects with QwpSendClosedError, which is not a QwpUpgradeError, so the failover sweep in createQwpFailoverConnectionFactory does not stop: it moves to the next endpoint carrying the same, now aborted signal. openQwpWebSocket saw signal.aborted, closed the socket and returned early -- but its open/message/error/close listeners are attached at the very end of the executor, so that socket had none. `ws` answers close() on a CONNECTING socket by emitting `error` on a later tick, and an EventEmitter with no `error` listener rethrows into the process. A catch around close() cannot stop it: the throw arrives after close() has already resolved. Record the pre-aborted signal instead and apply it after the listeners are attached, so the close it triggers has a subscriber and the promise rejects the way every other failure does. Also attach a throwaway error listener before the timeout-validation teardown, which tears down a CONNECTING socket that nothing has subscribed to yet for the same reason. Reproduced 10/10 before the fix and 0/25 after, driving the built ESM bundle from a fresh child process and reading the raw exit code. The regression test asserts the ordering directly rather than the crash, because `ws` defers the emit and a synchronous throw would only be swallowed by closeSocket()'s own try/catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The slot lock identified an acquisition by pathname and mtime, both of which are reused the instant a lock changes hands. Two consequences, each reproduced through QwpNodeFileReplayStore against real directories. A release removed the owner directory by path. Because a failed release is parked on a module-global list and retried before the next acquisition of any lock in the process, a stalled holder that later opened an unrelated journal deleted the owner directory of whichever process held that first pathname by then. A fourth process could then open a journal a third was already appending to. Observed: owner inode 1414684522 -> null, then "T opened slotA while R holds it". Staleness fell back to the `.lock.pid` sidecar when the owner record could not be read, and stamped it with the local hostname. The sidecar deliberately outlives its holder for Java parity, so it always names a process that has exited, and the fabricated hostname satisfied the same-host guard that was supposed to make a foreign PID meaningless. Every acquisition is briefly recordless, between its mkdir and its record write, so a contender arriving in that window judged a directory that had just been created stale and renamed it away from its live owner. The comment on that fallback already said a sidecar "can only expire by mtime"; the code did not. Write a per-acquisition token into the owner record, verify it before removing anything, and let a recordless directory expire by mtime alone as intended. Deterministic before/after on the mid-acquisition state: "ACQUIRED (stole it), ownerDir replaced" becomes "refused: QwpReplayStoreLockedError, ownerDir intact", with an owner record naming a live PID still refused in both. This does not change the documented mtime reclaim of a genuinely stale slot, which is covered separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A store-and-forward holder that stopped heartbeating kept appending to
its journal after another process had taken the slot over, destroying
frames the new owner had already durably appended.
Nothing on the write path consulted the lock. `compromised` was read in
exactly two places, both inside advisory-lock.ts, so the reclaim was
visible only at close() -- long after the damage. appendOnce() writes at
SEGMENT_HEADER_SIZE + logicalSize through an already-open handle, so the
resumed holder wrote at offsets the new owner had moved past, and a
frame's sequence is derived from its position in the segment: a
same-width overwrite leaves a journal that reopens with contiguous
sequences, valid CRCs, no torn tail, and no data-loss report.
Three changes make the loss impossible:
- assertReady() -- the chokepoint every mutating path already routes
through -- fails with the new QwpReplayStoreLockLostError once the
slot lock can no longer be vouched for.
- The heartbeat treats ENOENT as proof of loss. It previously waited
for a drifted mtime, which a removed directory can never produce, so
a lock whose directory was simply deleted was never noticed at all.
It also compares the acquisition token, because an mtime cannot
separate our directory from a replacement made inside the same clock
tick, and some filesystems only report whole seconds.
- Ownership expires on elapsed time, not only on the heartbeat firing.
The heartbeat is a timer, so the very block that loses the lock also
stops the timer that would notice; the first write after resuming
landed before it could run. This is conservative by design: the
holder gives up as soon as a contender could have taken the slot.
Two processes, real directories, a 20s main-thread block. Before: the
resumed holder's five appends all resolved, close() was clean, no
callback fired, and the byte census read A=1280 B=0 -- every one of the
new owner's durable frames gone. After: all five appends fail with
QwpReplayStoreLockLostError and the census reads A=640 B=640.
QWP.md documented the reclaim but not what the reclaimed holder then did
with its open handle, and claimed a second process cannot mutate journal
contents. Both paragraphs now describe the actual contract.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Omitting a nullish column took the rest of the call's validation with it. The guard added for issue #28 is the first statement of all 13 ILP setters, so a null or undefined value returned before the column name, the row state, or the decimal scale had been looked at. The result was a diagnosis that depended on the data rather than on the code: the same call site raised on rows that carried a value and stayed silent on rows that did not. A misspelled name, a name over max_name_len, a symbol placed after a column, or an out-of-range decimal scale could therefore first surface in production, on whichever row happened to be populated. On a sender with max_name_len=5 this whole sequence threw nothing and flushed "t i=1i\n": .decimalColumn(12345, null, 999) .arrayColumn("bad?name", null) .stringColumn("wayTooLongForMaxNameLen5", undefined) .symbol(123, null) .intColumn("i", 1) Move the value-independent checks into validateColumnCall() and validateSymbolCall(), run them ahead of the nullish guard, and drop them from writeColumn(), whose callers are exactly these setters -- so the name is still scanned once per cell, not twice. The reported error is now the real defect rather than the incidental one. Where this sequence previously reported "Column value must be of type string, received undefined", it reports "Column name is too long, max length is 5". Nullish values still omit the column, and a value the negotiated protocol version cannot represent is still skipped rather than rejected, which is the behaviour the suite already pins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
README states that passing null or undefined omits the column "on both the ILP and QWP senders, and to the compiled QWP writers". That held for 21 of the 22 QWP column methods. long256Column was the exception: its four value parameters were plain bigint, so BigInt.asIntN() raised "Cannot convert null to a BigInt", failRow() discarded the row, and a plain-JavaScript caller mapping an optional field onto it got a raw TypeError where every sibling method omits the column. TypeScript callers were spared, which is why no suite noticed. Accept nullish words. A LONG256 is one value spread over four arguments, so "no value" means all four are absent; that omits the column. A partial set is a caller mistake rather than a NULL and now says so, instead of failing inside BigInt conversion. Also correct the one other place the shared documentation overstated the rule. Sender.decimalColumn's TSDoc said "An empty array represents the NULL value" for both backends, but the ILP buffers write an explicit NULL decimal field for an empty Int8Array while the QWP sender omits the column, as it does for null. Both land as NULL for a column that already exists -- verified against QuestDB 9.4.3 and 10.0.1-nightly -- but the encodings differ and the TSDoc now says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two ways a SYMBOL column could reach QuestDB as empty strings, with the
frame acknowledged OK.
encodeQwpIngressFrame() accepts a bare numeric dictionary ID for a
SYMBOL value: symbolId() has an explicit branch for it. Its non-delta
twin, symbolText(), had none. It read `.text` off the number, got
undefined, and TextEncoder encodes undefined as zero bytes -- so every
distinct symbol in the frame collapsed into a single empty-string inline
dictionary entry with all rows indexing it. The non-delta encoder builds
its dictionary out of the texts, so there is genuinely nothing to
resolve an ID against; say so rather than emitting a frame that looks
valid. The delta path, which is handed the dictionary that gives IDs
meaning, is unchanged.
QwpNodeUdpSession.sendTables() takes QwpIngressEncodeOptions but
encodeUdpDatagrams() dropped the argument and hardcoded
`{ gorilla: false }`. A caller who correctly supplied a delta dictionary
had it silently ignored and fell into exactly the case above. A datagram
has to decode on its own, so a connection-scoped dictionary cannot apply
to one: reject `dictionary` and `confirmedMaxSymbolId` instead of
accepting and discarding them, and pass `gorilla` through rather than
ignoring that too.
The high-level QwpSender coerces symbol values with String(), so it was
never affected; this is the low-level `./qwp` surface that QWP.md
documents for advanced integrations.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The egress RESULT_BATCH decoder bounded the table- and column-name wire fields with QWP_MAX_TABLE_NAME_LENGTH and QWP_MAX_COLUMN_NAME_LENGTH. Those limits are UTF-16 code-unit counts -- the unit Java's TableUtils measures in, which identifiers.ts mirrors on the ingress side -- but the wire field they were compared against is a UTF-8 byte count. So this client could encode identifiers it was then unable to read back. No configuration was needed: at the default limit of 127, a name of 64 accented characters is 64 code units and 128 bytes. Ingress accepted it, and a later query carrying that name failed with "column name length out of range: 128". That QwpProtocolError is routed to recoverProtocolFailure, which replays the same query on a replacement connection, so it reproduced on every endpoint and ended in QwpReconnectExhaustedError with the whole failover set deprioritized. Bound the decode by QWP_MAX_IDENTIFIER_BYTES instead: the same limit expressed as the widest UTF-8 encoding of a maximum-length identifier, three bytes per code unit. The allocation stays bounded, which is what the cap is for, and every identifier the encoder can legally produce now decodes. Also record the unit in the QWP.md `max_name_len` row, since a limit whose unit is unstated is what allowed the two sides to drift apart. Note that `max_name_len` still has no upper bound, for servers configured with a larger cairo.max.file.name.length; setting it above the protocol identifier limit remains the operator's business to match to their server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two ways the three new subpaths were published without being usable. typedoc.json listed only ./src/index.ts, which exports none of the QWP surface, so the generated API reference -- the site GitHub Pages serves from docs/ on main, and the package homepage -- documented 1 of the 324 symbols the QWP entry points export, and that one was a Sender.symbol name collision. Typedoc had been reporting it all along as "referenced by ... but not included in the documentation". Adding the three entry points takes the output from 17 pages to 349, with a module page each for qwp, qwp/browser and qwp/node. TypeScript's node10 resolution ignores `exports`, and `module: "commonjs"` implies node10 unless moduleResolution is set explicitly -- which is what `tsc --init` still emits. Such a consumer got TS2307 for all three documented imports while the same imports worked at runtime. Declaring them in `typesVersions` as well fixes it: verified against a real `npm pack` install, three TS2307 errors before and none after, across node10, node16, nodenext and bundler. The build-artifact check now walks the typesVersions targets too, since a missing one breaks compilation in a way no runtime suite can see. Note that this proves the files exist, not that they resolve -- both tsconfig.dist-types*.json use explicit `paths`, which bypasses module resolution entirely, and that is why neither caught this. A real guard needs a packed install. docs/ itself is not regenerated here; it is refreshed at release, and is already a version behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two halves of one lifecycle gap. close() could not cancel a first connect. The reconnect loop owns an AbortController, but the initial attempt bypasses it -- it is either handed to QwpReconnectingIngressConnection as `initialConnection` or awaited directly -- and both were built by calling the factory with no signal. closeNow() could therefore only attach `.then(c => c.close())` to the pending promise, so the socket and its opening deadline outlived close() by the full connect/auth timeout. A CLI, serverless or test process that closed a sender and expected to exit hung for up to that long. Measured against a peer that accepts TCP and never answers the upgrade: close() returned at ~305ms and the process exited at 20008ms. QwpSenderSessionFactory now takes an optional AbortSignal, mirroring QwpConnectionFactory (whose doc comment already notes that factories ignoring the parameter stay assignable), and the sender aborts it when a close finds a connect still in flight. Same probe: exit at 309ms. close() also bounds its own flush with a deadline but cannot cancel it, so an abandoned close flush stayed runnable. getSession() clears sessionPromise when a connect fails, so that leftover flush reached the cleared field, dialled the database again and wrote rows after close() had already returned -- an application closing a sender to stop writing kept writing. Through a proxy that swallows the first connection, the server received an ingress frame 366ms after close() rejected with QwpSenderCloseTimeoutError; it now receives none. The new guard is keyed on `closed`, not `closing`: close() is documented to publish completed rows, and doing that legitimately needs a session even when none was opened yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The journal's exclusion, reclaim and release rules are entirely about what separate processes observe of each other, and nothing tested them that way. Every existing lock test runs two stores in one process, where they share a module-global pending-release list, one event loop and every advisory-lock object -- so the mechanisms could be observed in isolation but never the contract. The closest existing test, "arbitrates acquisition over stale Java lock metadata", also builds its directory with mkdtemp, so the parent has no .slot-locks/ and no .lock.pid from an earlier producer: the state a contender actually meets in production is structurally unreachable there. Adds a suite that forks real producers against the built package, since that is what a deployed process runs. It covers exclusion under contention from a used parent, a live heartbeating holder refusing a contender, adoption of a SIGKILLed producer's slot with its frames recovered, a reclaimed holder refusing to write, and a stalled holder's release leaving a live lock alone. Two of the five fail against the code before the lock fixes; the other three passed already and are labelled in the file as contract tests rather than regression tests. Staleness is produced by backdating the owner directory's mtime rather than by waiting out the 15s window, which is the same on-disk state a paused holder leaves and keeps the suite to about 21 seconds. It runs under `pnpm test:dist`, which build.yml already executes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
authTimeoutMs inherits connectTimeoutMs when it is not set, deliberately, so that narrowing only the connect deadline is not exceeded 75x by a default nobody chose when a peer accepts TCP and never answers the upgrade. Three places described the two budgets as independent instead: QwpNodeWebSocketOptions.authTimeoutMs said "Defaults to 15s", and two QWP.md passages said each deadline "independently" bounds its phase and that an attempt can take up to their sum. Measured, connectTimeoutMs=300 alone fails the upgrade at ~308ms, not at the 600ms those passages imply, so an operator narrowing connect_timeout to fail fast on DNS was also narrowing the upgrade deadline without being told. The behaviour is right and is unchanged; the connect_timeout table row and the connection-lifecycle section already described it correctly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
Three files this branch added carried formatting Prettier rewrites: ingress-session.ts's acknowledgementFailure signature, two statements in core.test.ts, and a decimal-scale assertion in sender.buffer.test.ts. `pnpm format` writes but nothing verified the result, which is how the drift survived into files no other gate reads. Add `pnpm format:check` over the same glob `pnpm format` writes, so the two cannot disagree, and run it in build.yml beside eslint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
GitHub Pages serves main:/docs directly and no workflow regenerates it, so the committed tree is the published site. It was last rebuilt before roughly twenty commits that touched packages/*/src, leaving nine pages missing -- among them QwpIngressAckAbandonedError, QwpMemoryReplayBatchTooLargeError and QwpReplayStoreLockUnprovableError, which test/qwp/public-api.test.ts names as required public exports and which callers have to catch by name. Regenerated after the fixes in this branch, so it also carries the corrected authTimeoutMs and backpressurePolicy documentation. All 204 runtime exports of @questdb/nodejs-client now have a page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
…verdicts Three defects in the Node store-and-forward journal, all found by executing against real directories rather than by reading. Two worker threads sharing one `sf_dir` both took its advisory lock, and the second reclaimed the first's live owner directory. `PROCESS_INSTANCE` is minted per module registry, and every worker loads its own copy of the module, so a live sibling presented the same shape a dead predecessor does: our PID, alive, with an instance we did not mint. `isReusedPid` treated that as proof. It is only a necessary condition now; the heartbeat is the proof, so a same-PID successor waits out one staleness window instead of adopting the slot at once. Measured before the fix: 60 successful flush() calls, then a successor's recovery found 41 records with all 20 of the second writer's gone, two producers sharing one frame-sequence space, 14 frames queued for duplicate replay, and one symbol ID resolving to two different strings. A load that failed unwound in a finally that could throw, so a fault in the teardown -- EMFILE, EIO, an NFS ESTALE, the class QwpReplayStoreLockUnprovableError exists for -- replaced the recovery verdict. isQuarantinableReplayRecoveryError then said no, and the corrupt slot was never moved aside, so the producer could not start on any later restart. The unwind now runs for its side effects only, the way close() already handles its own. Recovery stamped MANIFEST_REQUIRED_FLAG onto every surviving segment, including the empty active one it had just proved carries no flag -- and that flag is the entire basis for reading "empty" as "records were lost". The next restart read back evidence recovery had forged and reported a data loss that never happened. The stamp now belongs to the next append, one syscall before the first record, as it already does after activateHotSpare. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
Two opposite misclassifications in the same loop, both reachable from a documented connect string. A permanently invalid option was retried. Option validation runs inside the per-attempt connection callback, so the loop met a bad `connectTimeoutMs`, an out-of-range `maxVersion`, endpoint userinfo or an Authorization conflict the way it meets a refused connection: the classifier retries anything carrying no structural `retryable` flag. Measured on the shipped ingress defaults: 125 attempts over 300 seconds, no log output for the whole window, and then a generic QwpReconnectExhaustedError whose cause named the elapsed deadline rather than the option -- the diagnosis was unrecoverable from the thrown error, its causes and its stacks. Under `lazy_connect` it never ends at all: connect() resolves, rows accumulate, nothing is ever sent. Retrying cannot fix an option, so these now say so and surface in one attempt, as the equivalent connect-string spelling already did. A transient rejection latched a running producer terminal. The retry-forever exemption for endpoint policy failures was gated on having connected once, so the first attempt of a deferred-connect sender went terminal on any upgrade rejection -- including a 404, which the connector itself marks `tryNextEndpoint` because one node returns it mid-deploy while its peers are healthy, and which the orphan drainer already treats as transient. connect() had resolved and the first flush() had been accepted by then, so this ended a running producer, and under the documented `lazy_connect` default of memory replay it dropped the frames with it. Only a rejection the whole cluster would repeat stays terminal now; 401 and 403 are unchanged, and their pinning test still passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
QwpUpgradeError.url and QwpFailoverError.attempts[] strip userinfo from an endpoint before a caller can log it, and test/qwp/session.test.ts pins that. The reconnect events carrying the identical string did not: connected, reconnecting, reconnected, failed-over, attempt-failed and the egress onReplayReset all passed the factory's URL through verbatim, and the documented way to use an event sink is to log the whole event. A credential therefore reached the browser console and any telemetry behind it -- a channel with a different retention and access profile from the application's own config. The Node entry point rejects userinfo outright, so its own transports could not produce this. The browser package has no such guard, and a caller-supplied QwpConnectionFactory reaches it on either runtime -- which is the exact case the redaction helper's own comment says it exists for. Verified in real Chromium: a credentialed ws:// URL constructs, keeps its userinfo in .url, and opens. Both dispatchers now redact at emitEvent, so every event kind is covered by construction rather than per call site. The helper moved to _internal/ because qwp/index.ts re-exports transport.ts wholesale, and an internal fix should not grow the published surface of either package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
…atic path
Two validations ran only when a QWP sender was built from a connection string,
so the same configuration written as a plain options object was accepted and
quietly ignored. `validateUdpSecurityOptions` was already applied on both
paths; these two were the ones left behind.
The six ILP-only keys -- init_buf_size, max_buf_size, request_timeout,
request_min_throughput, retry_timeout, stdlib_http -- are rejected by `udp::`
with a precise diagnostic, and by `ws::`/`wss::` because the QWP schema knows
no such key. `new Sender({protocol: 'udp', ..., max_buf_size})` took them and
dropped them: a caller capping memory got no cap and no warning, which is the
exact condition the connect-string check was written for. The check now covers
ws/wss as well as UDP and runs on both construction paths.
The programmatic `wss` path also accepted a root CA together with verification
disabled, leaving the CA inert and the connection unverified where the
documented connect string calls that combination an error, and it read the file
with a bare readFileSync -- so a path to something that is not a PEM bundle was
installed as a trust store and only failed later as an opaque TLS error at
connect time. Both now follow the connect string, and the PEM diagnostic names
whichever key the caller actually wrote. The ILP https/tcps transports are
untouched; this is the new wss surface only.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
activateHotSpare() writes a segment's base sequence, fsyncs the manifest that names it, and only then stamps the manifest-required flag. A crash or a write fault in that window leaves a real segment on disk carrying a non-zero base, no records and no flag, and recovery is required to retain exactly that shape rather than forge the flag it did not find. Retaining the base was the problem. Once the journal has drained -- the steady state of a producer that is keeping up -- the ACK watermark is gone and a reconnecting transport restarts its frame numbering at zero, so the retained base belonged to a numbering nothing else remembered. The segment contiguity check in appendOnce() then rejected every frame the producer offered, non-retryably and identically after every restart, while load() went on resolving successfully and reporting no data loss. Nothing healed it: quarantine only fires on a corrupt load, and the orphan drainer skips both live slots and flagless empty segments. The producer kept accepting rows that could never be journalled. An empty recovery now retires that segment with its manifest. Every segment surviving the scan carries live records except the single empty active one recovery may retain, so no recovered entries means the segment is provably record-free -- that is what retaining it means -- and it holds no manifest-required flag, so retiring it drops the stale origin without discarding a frame or forging the evidence the flag stands for. Re-basing it in place is not the alternative: the manifest pins each segment's base and writeManifest() refuses to move a boundary backwards, so a rewritten base would fail the next load as corruption. Already-wedged directories recover on their next open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
QwpNotificationDispatcher waits for an `async` handler's promise before it dispatches the next notification, so a slow observer applies backpressure to its inbox and loses the oldest entries. Four of its call sites hand it the user callback directly and get that. The two inside QwpIngressSession queued thunks that called safelyInvoke(), which returned void, so the dispatcher saw every notification finish immediately and re-entered the observer once per drain turn. An `async` onProgress, onError or onSenderError therefore accumulated concurrent copies -- eight frames produced eight live observers in a test that expects one -- while droppedProgressNotifications and droppedErrorNotifications stayed at zero. QWP.md tells operators to read a non-zero value there as "an observer is not keeping up", so the published health signal read healthy precisely when the observer was furthest behind. The same onSenderError callback was serialized when it reached the orphan drainer's inbox and not when it reached this one, making the behaviour depend on the transport rather than on the user's code. safelyInvoke() now returns the contained promise. It is the already-handled one, so it never rejects and the many callers that ignore it keep exactly the containment they had. The error path builds up to two observers per notification, so it waits for both rather than reporting one and leaving the other to run underneath the next notification. Left alone: the orphan-recovery wrapper in qwp.ts, which discards the promise deliberately and says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
Two ways the shared ws::/wss:: vocabulary reached a Sender with the wrong scope attached. The failover keys. QWP.md scopes `failover`, `failover_max_attempts` and the three failover backoff/duration keys to egress, and parseEgressReconnect() is their only reader, so they resolve into `egressSession` -- the section a Sender discards, and the very one `initial_credit` lands in. They were nevertheless left out of the ignored-key warning on the premise that ingress honours them. Ingress honours the other keys named in that premise; it reads none of these. So a Sender handed a cluster string that tuned failover applied none of it without a word, while the pool key beside it warned, and `failover=off` did not disable ingress endpoint sweeping either, because that sweeping is unconditional. They now warn with the rest of the egress section. The closing handshake. Opening runs under two deadlines and authTimeoutMs already inherits an explicit connectTimeoutMs, so a caller who narrowed the connect budget is not held for the 15s default when a peer accepts TCP and never answers. Closing is a handshake too, and a peer that accepted the upgrade and then stopped reading never answers the close frame, so close() ran to the full closeTimeoutMs default. The pool's shutdown deadline does not bound it -- the await that fires terminate() runs before that deadline is consumed -- and closeTimeoutMs has no configuration-string key, so acquire_timeout_ms, the only shutdown budget a connect string offers, governed a borrowed slot and not an idle one. A caller asking for 200ms waited 15s. closeTimeoutMs now inherits an explicit connectTimeoutMs the same way; set it to decouple them. Neither default changes when nothing is set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
`addr` is host[:port], so userinfo in it is always a rejected value -- but rejecting it interpolated the whole entry into the thrown message, and a connect string is parsed at startup, where that message is exactly what a caller's configuration logging writes out. `ws::addr=admin:s3cr3t@host:9000` produced "Invalid QWP cluster address: 'admin:s3cr3t@host:9000'", through parseQwpNodeClientConfig() and Sender.fromConfig() alike. One of the seven rejection sites even tests for userinfo before throwing, so it identified the credential and printed it anyway. The rest of the client already follows the opposite rule, three times over: redactQwpEndpoint() for endpoints reaching failover errors and reconnect events, redactedUrlText() for the browser bootstrap's own validation errors on caller-supplied endpoints, and endpointUserinfo(), which reports "a password" rather than the value. Even this file refuses a colon-bearing username without echoing it. Every address rejection now drops what precedes the last `@`, which cannot appear in a valid entry, and keeps the host and port that make the error actionable. The port rejection below them is reached only after a digits-only test, so it can carry nothing secret and is left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
…ds back QWP.md documents `acknowledgement` as optional: await `publication` before releasing retryable source rows, and `acknowledgement` only when server acceptance is also required. The session rejects it anyway, from two paths a caller who followed that sentence never asked about -- the ACK deadline in startFrameWithPublication(), and rejectAll() in closeNow(). Under Node's default unhandled-rejection mode an unobserved rejection terminates the process, so the documented pattern killed the producer roughly ackTimeoutMs into any outage, and again on close() with a frame still in flight. An onError observer does not prevent it; the rejection is a different promise. QwpSender never hit this because it already applies the containment to its own use of these methods, and the durable-ACK poll does the same. The public API was the one left holding an unobserved promise. Attaching a handler settles Node's tracking without consuming anything: the same promise is returned, so a caller who does await it still receives the rejection, and both rejecting paths keep reporting through recordError() and the onError observer. All three returned acknowledgements need it -- the per-frame one, the split-batch aggregate, and the delta aggregate, the last of which also carries the publication rejection. The two aggregates build fresh promises with Promise.all(), so the per-frame containment does not reach them. Reproduced before the fix against a server that accepts the upgrade and never answers: `sendFrameWithPublication()` then `await result.publication` exits 1 at the ACK deadline, and again on `close()`. Both exit 0 now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
… zero `queryFlags` is typed `number | bigint`, and the trailing flags varint is appended only when it is non-zero. The test was `(request.queryFlags ?? 0) !== 0`, which is true for `0n`, so the two spellings of "no flags" produced two different frames: 0 and an omitted field appended nothing, 0n appended a trailing 0x00. One declared value, two encodings. encodeQwpQueryRequest() is a public export of both packages, reached through `export * from "../_qwp/_core"`. Whether QuestDB tolerates the extra byte is not the point -- it may well read it as zero -- but a caller accumulating flags as bigints should not have to know which zero to write. The session path never hit it: encodeQueryRequest() passes QWP_QUERY_FLAG_RESET_DICTIONARY or undefined, both numbers. Rejecting both spellings keeps writeQwpVarint() as the validator for everything else, so a negative, non-integer or out-of-uint64 value still raises the same RangeError it always did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
…e programmatic path
Three keys were taken and dropped when a QWP sender was built from an options
object rather than a connection string. A ws::/wss:: string is parsed by the
QWP schema, which rejects each of them with a relocation hint, and a udp::
string is rejected by parseProtocolVersion(); neither parser runs on the
programmatic path, and nothing downstream reads the keys.
new Sender({protocol: 'ws', ..., max_datagram_size: 1400}) -- accepted, unread
new Sender({protocol: 'ws', ..., multicast_ttl: 1}) -- accepted, unread
new Sender({protocol: 'ws', ..., protocol_version: '2'}) -- accepted, unread
new Sender({protocol: 'udp', ..., protocol_version: '2'}) -- accepted, unread
This is the gap bee2458 closed for the six ILP-only keys, in the same
validator, left open for the transport-scoped ones. The TSDoc for
max_datagram_size and multicast_ttl already claimed ws/wss reject them, so the
documentation described the connect string and not the API.
parseProtocolVersion() also stopped defaulting ws/wss to protocol version '1'.
It fell through to the ILP branch and stamped a value nothing then read; it now
rejects an explicit one and returns, the way it already did for udp.
The ws/wss half of bee2458's own check had no test -- removing the
validateQwpUnsupportedOptions() call from createConfiguredQwpSender() left the
suite green -- so the new case covers that too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
…verflows writeColumn() sets hasColumnCall after its own checkCapacity(), then restores it in the catch if the value encoder throws. Only the first half was tested. "keeps the symbol section open when a column call overflows the buffer" stops at the outer checkCapacity(), which rejects before the flag is set, and "leaves no partial cell behind when a column value overflows the buffer" does reach the catch but asserts only the position -- and its preceding intColumn() has already set the flag, so restoring it changes nothing there. Removing `this.hasColumnCall = hadColumnCall` therefore left all 74 buffer tests green, while a caller who handled the overflow and fell back to a symbol got a second, unrelated "Symbol can be added only after table name is set and before any column added" on a row where nothing had been written -- a call base accepted, since base never set hasColumns either. The new case is the first column on the row, with a short name so the separator and name fit and the value encoder is the thing that throws. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
…pplies, and the v3 scale change Three gaps between what the packages do and what a reader can find out. The error table listed 25 of the 45 exported error classes, while QWP.md's own public API policy says the compatibility contract covers "errors". Eight of the missing ones appeared in neither the table nor public-api-contract.ts -- QwpSendClosedError among them, constructed on 39 paths including the browser entry point, and QwpIngressSessionClosedError, which is what an abandoned acknowledgement rejects with. A consumer writing catch policy from this document had no entry for errors the client really throws, and renaming one would have failed no gate. All 45 are now listed and contracted, and a test keeps the table exact in both directions -- it found the 45th, QwpBrowserSessionBootstrapError, which is exported from the browser package only and which the manual sweep had missed. Twelve keys showed "-" in the Default column while the code applied a concrete value: initial_credit 0, buffer_pool_size 4, the four pool bounds, the four pool timeouts, and the two inbox capacities. The three reconnect_* keys and the four failover_* keys had no default documented either; the reconnect ones differ by side, so they now carry both, ingress first. A test pins each documented number to its source constant, reading the pool bounds back from a default client's own metrics because those constants are module-private. max_batch_rows, compression_level, client_id, the credentials and the rest keep their "-": they genuinely have no client-side default. The v3 decimalColumn() scale check tightened in this PR and was not in the migration notes. `scale` is typed `number` and a non-integer one used to reach Buffer.writeInt8, which coerces rather than rejects, so 2.5 was written as scale 2 and NaN as scale 0 -- the row went out with a scale the caller never asked for. Rejecting it is the fix, but it turns a previously accepted, type-valid call into a throw, so it belongs beside the nullish note. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
The action scans a sliding 30-commit window of the pull request, so as
commits land the window moves. It has now reached the TCP auth test that
spells out a second P-256 scalar, which `generic-api-key` scores at 5.05
entropy; the previous push passed only because its window stopped short
of that commit.
Editing the test cannot fix it. The scan reads `git log` patches, so the
value stays in the diff of the commit that introduced it however the file
looks today. Add a config instead: extend the default rules, and allowlist
the exact sample values the tests, examples, and README have to spell out.
Every one is a placeholder or the published documentation keypair, handed
to a mock server inside the test process.
Allowlisting by value rather than by rule or path keeps the rest armed --
a genuinely new secret written in the same `{ keyId, token }` shape is
still reported.
Verified against gitleaks 8.24.3, the version the action pins: the window
that failed and all 351 commits of the branch now report no leaks, and a
planted secret in that same shape is still caught.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017dABiQabtxr6yL4Ufp7RaS
The browser client decided durable acknowledgements from socket.protocol alone, and raised QwpDurableAckUnavailableError when the server had not echoed questdb.qwp.durable-ack.v1. That check cannot run in a browser: the WHATWG "establish a WebSocket connection" algorithm fails the connection when the client offered a subprotocol and the response names none, so the socket never opens and the callback that inspects socket.protocol is never reached. The purpose-built error, and the capability-gap settle budget in applyDurableAckMismatchPolicy behind it, were unreachable through the one runtime they exist for. The server now echoes the token whenever it was offered and reports the capability on the ingress SERVER_INFO frame instead, so this reads the verdict from there. decodeQwpIngressServerInfo returns QwpIngressServerInfo rather than a bare batch cap, and requires exactly six bytes: a five-byte frame comes from a server that predates the capability byte, and reading it as "durable ACK off" would turn a version skew into a wrong answer on the one field nothing else can re-derive. applyQwpBrowserIngressHandshake folds the bit into the handshake and raises QwpDurableAckUnavailableError from an open socket, which lets the existing mismatch policy run. The two connect paths shared a duplicated handshake callback, so browserIngressHandshake now holds the single remaining upgrade-time check: a missing echo means the server does not speak the browser negotiation at all, which Node and an injected webSocketFactory can still observe. connectQwpBrowserRawEndpoint consumes the frame too, because offering the token is what makes the server send it and an unconsumed frame would surface to the caller as a data message. Requesting durable ACK alongside ingressNegotiationTimeoutMs: 0 is now an error rather than a guess, since that combination asks for a capability whose verdict only arrives in the frame while asking not to wait for the frame. connectQwpBrowserRawEndpoint validates that option through the same helper as its sibling, which it did not before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ak82JZCcANpEtK85fvnUgK
A maintenance batch checked directory ownership once on entry and then ran up to TRIM_BATCH_SIZE trims, each with a handle close, an fsynced manifest write and an unlink. `QwpNodeAdvisoryLock.lost` turns true on a timer -- a heartbeat that cannot read the owner record deliberately lets `provenAtMs` go stale, and an event-loop stall past the liveness window does the same -- so the lease could lapse part-way through the batch, with no second process involved. Past that point writeManifest() silently skipped its write while trimSegment() unlinked the segment anyway, leaving sf-manifest.bin naming a file that no longer existed. validateRecoveredManifest() rejects that pair and runs before rewriteManifestForCurrentSegments(), so neither this process nor a successor could repair it: the next load failed twice, isQuarantinableReplayRecoveryError() sent the whole slot to quarantine, and every frame the server had not acknowledged yet was abandoned -- over segments that were fully acknowledged and whose deletion mattered to nobody. writeManifest() now reports whether the on-disk manifest describes the boundaries it was asked for, and trimSegment() deletes nothing when it does not. runMaintenanceBatch() re-checks ownership per trim rather than only on entry, which also covers the trimSegment() call activateHotSpare() makes directly.
Reading the durable-ACK verdict from SERVER_INFO moved the answer out of the subprotocol echo, which the server now sends whenever the token was offered. The fake-socket suites were updated with it -- session.test.ts passes ingressServerInfo(cap, true) and core.test.ts pins byte 0 to false and byte 1 to true -- but the real-browser mock still sent the frame with the capability bit clear while the test asserted durableAckEnabled: true. The test was internally self-contradictory and the qwp-browser CI job failed on it, so set the bit the assertion expects. Production is the correct side of this: applyQwpBrowserIngressHandshake() has no other source for the verdict a browser can read.
Two gaps in what the suite actually held, both proved by mutation: every regression below left `pnpm test`, `test:dist`, `check:packages` and all three `typecheck:dist` configs green. The public-API contract only asserted that each documented name was present, so it pinned nothing against additions. Both roots re-export the shared barrel with `export *`, which means any new `export` under `_qwp/**` became public API on two published packages, under semver, unreported -- package-boundaries.e2e.ts pins the `exports` subpaths in package.json rather than the module's names, and public-api-contract.ts pins types. The contract arrays are now the complete export inventory and are compared in both directions, so widening the surface is a deliberate edit with a reviewable diff. That inventory also covers the ILP names on the Node root, including the newly exported SenderBufferV3, whose removal nothing caught. It found one omission already: parseQwpNodeClientConfig was exported but undocumented. The QWP-shaped members on the root Sender -- writer(), flushAndGetSequence(), publishedSequence, acknowledgedSequence and waitForAcknowledged() -- were only ever driven through a `ws::` sender, leaving the branch every HTTP/TCP user takes untested. Dropping the flush() from flushAndGetSequence() is silent data loss: the call resolves, the caller believes the rows were sent, and only close() mentions the unflushed buffer.
Four unrelated defects, each with a regression test that fails without its fix. Store-and-forward recovery reads a manifest-required segment holding no records as proof that records were written and lost. activateHotSpare stamped and fsynced that flag, then ran a whole trimSegment of the previous segment -- a manifest write, its fsync, a directory fsync and a cross-thread unlink -- before appendOnce wrote the first record, so a kill anywhere in that stretch forged the verdict and reported abandoned data to a producer whose first append had not returned. The stamp moves to the append, immediately before the record write, which is the ordering the flag has always been documented to have and the one recovery already applies to a retained empty active segment. A wss upgrade agent was accepted only when it was an instance of https.Agent. That tests inheritance rather than whether the agent can serve the scheme, so it refused every tunnelling agent -- the shape https-proxy-agent, socks-proxy-agent and proxy-agent all build on -- while ws given the same agent completes the upgrade. Node applies the real check inside https.request and reports a mismatch as ERR_INVALID_PROTOCOL, so wss defers to it; ws keeps its own check. That error is raised synchronously from the ws constructor and carries no retryable flag, which the reconnect classifier treats as retryable, so it is now marked. The orphan-recovery reconnect wrapper discarded what safelyInvoke returns, so the notification inbox could not tell an async observer from a finished one. The caller's own callback was serialized on a foreground session and re-entered on an orphan-drained one, and the inbox bound never engaged because its queue was drained on the same turn. The wrapper returns the promise, which safelyInvoke documents as already contained and non-rejecting. It moves to qwp-node/ so a test can reach it without widening the published surface. Egress incremented a notification drop counter that nothing could read. attempt resets on every success, so a delivered stream that lost events reads exactly like a healthy one. QwpEgressSession.metrics reports it, as ingress does. The public API contract pinned only runtime exports, so deleting the SenderBuffer, SenderTransport and TimestampUnit re-export lines removed them from the published declarations with every gate green. They are named in the contract now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ejquoL94xKsYAfpbRGkri
Five defects, each with a regression test that fails without its fix. The reconnect loop's backoff wait sat behind `backoffMs > 0`, and that wait is the loop's only macrotask. initialBackoffMs 0 is a value validateReconnectPolicy accepts and this suite passes in 62 places, so a connection attempt that fails without an I/O turn -- a caller-supplied webSocketFactory, or a browser WebSocket constructor throwing SecurityError on mixed content -- left both the ingress and the egress loop spinning in microtasks. Timers, I/O and close() all stopped for as long as connecting kept failing, which under Node store-and-forward is forever, since that policy reconnects unbounded. The wait now runs on every retry and takes a zero delay when backoff is off; only a non-zero backoff doubles. maxBackoffMs 0 reached the same spin by driving backoff to zero after the first retry. Deferring a wss upgrade agent to node fixed validateQwpWebSocketAgent but left selectQwpSchemeAgent testing `instanceof https.Agent`, so the same object was accepted through qwp.webSocket.agent and dropped, with one warning, through the top-level `agent` option -- and a proxy-only deployment then connected direct. That path admits any http.Agent on wss now, which is what a tunnelling agent is, and keeps its own check on ws, where an https.Agent would attempt TLS on a cleartext socket. An agent that genuinely cannot serve wss still fails by name, because node raises ERR_INVALID_PROTOCOL and connectQwpNodeEndpoint marks it non-retryable. node:dgram discards the completion callbacks of sends still queued in the handle when close() runs, and the UDP session resolved a send only from that callback, so a sendTables() racing a close() returned a promise that never settled -- and sendDatagrams() awaits each datagram, so one dropped callback stranded the whole call. The session tracks what it hands the socket and settles it on close: a datagram already queued resolves without advancing the watermark, the way a datagram the kernel refused does, and the rest of an interrupted batch fails with the closed error. The fake socket modelled the wrong half of this and now drops the callbacks too. The browser package's top-level `browser` field named the CommonJS build while `module` named the ESM one, so a resolver that prefers `browser` and ignores `exports` picked a build it cannot tree-shake, costing 231 KB of the 630 KB bundle. Removing the field sends those resolvers to `module` and leaves a CommonJS-only one on `main`; pointing it at the ESM build instead would hand an .mjs to the jsdom-style resolvers that read it and emit require(). THIRD_PARTY_NOTICES described the bundled fzstd as stock 0.1.1. It is patched to enforce the decompressed size a Zstandard frame header declares. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ejquoL94xKsYAfpbRGkri
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a complete QWP client surface that works in both browsers and Node.js while leaving the existing ILP transports Node-only.
QWP support ships as a preview:
QWP.mddocuments the compatibility baseline for the first QWP release, and imports from internal source paths are never supported.Entry points
The repository now builds two published packages from a shared private core, each exposing its complete API from its package root.
@questdb/nodejs-clientSender(including QWP ingress selected withws::,wss::, orudp::), QWP codecs, egress, TLS, and persistent store-and-forward@questdb/browser-client@questdb/nodejs-clientkeeps the existing Node.js transports and dependencies, so nothing about the current client changes for existing consumers.@questdb/browser-clienthas no Node.js imports, Node.js typings, Node engine requirement,undici, orws, so supporting browsers does not require compromising the Node.js build.Ingress
Senderintegration with fluent rows, batching, byte/interval auto-flush, commits, transactions, and ACK watermarkssender.writer(table, schema).row({...})) for repeated rows on one schema, with the full QWP column-type setudp::, Node-only) behind the same fluent row APIEgress
Observability
onProgress,onError, and the Java-parityonSenderErrorrejection stream for event-driven telemetryAPI and platform integration
qdb_sessionbenchmarks/) covering encoder floors, the high-level sender, egress views, store-and-forward persistence policies, and a live end-to-end laneQwpBrowserSessionAuthTest,QwpIngressUpgradeProcessorOnHeadersReadyTest, andQwpEgressMaxBatchRowsTest, plus the Enterprise REST/OIDC login suitesCompatibility
http/https/tcp/tcpssenders are unchanged, with the two exceptions below.auth: {keyId, token}supplies only the private scalar, and the JWK was completed with a hardcoded public point unrelated to it. Node.js accepted that inconsistent pair without validating it up to v24 and rejects it from v26 withERR_CRYPTO_INVALID_JWK, so TCP auth failed outright on that runtime. The point is now derived from the private key. Signing only ever used the private scalar, so signatures, credentials, and auth outcomes are unchanged on every Node.js version; callers passing a completejwkobject were never affected.nullandundefinednow omit the column, which QuestDB records as NULL; most column methods previously threw a type error. On protocol v2 this also changes the wire bytes forarrayColumn(name, null), which used to emit an explicit NULL-array marker — QuestDB rejects that encoding withARRAY_INVALID_TYPE(verified against 9.4.3), so omitting it is itself a fix. A row whose every value is nullish now fails when the row is closed rather than at the column call. Code that relied on the throw as a data-quality guard should validate before calling the sender.flock/LockFileEx; the Node.js client uses a pure-JavaScript directory lock and cannot participate in those kernel locks, so neither sees the other. The persistence format stays cross-client for sequential handoff — a directory written by one runtime can be opened by the other once the first has closed it — and two Node.js processes still exclude each other. Depending on a native addon for kernel locks was the alternative, and it left store-and-forward broken on any platform or Node.js major without a prebuilt binary.Dependencies
ws(Node WebSocket transport). There is no native dependency; store-and-forward locking is pure JavaScriptfzstdis bundled into the build output for egress decompression;THIRD_PARTY_NOTICES.mdrecords its licenseResolved issues
Null or undefined column and symbol values are omitted across the existing ILP senders and the new QWP senders.
Fixes Client should skip columns if value is null #28
The new QWP sender introduces the sender.write().row() API.
Fixes State-machine builder #60
Validation
pnpm vitest run benchmarks): 3 files / 14 tests passedpnpm typecheckpnpm typecheck:qwp-browserpnpm typecheck:testpnpm typecheck:benchpnpm eslintpnpm lint:benchpnpm buildpnpm test:dist(loads both built packages through theirexportsmaps): 3 files / 30 tests passedpnpm typecheck:distpnpm check:packagesDependencies and provenance