[BCN] Streaming improvements#4155
Open
leolambo wants to merge 8 commits intobitpay:masterfrom
Open
Conversation
Unify ExternalApiStream.onStream and Storage.apiStream into a single route-layer helper. CSPs can now return Readable streams while routes handle HTTP framing, error injection, and client disconnect cleanup. Supports JSON-array (default) and JSONL output. Pre-data errors reject so the caller can send a proper 5xx; mid-stream errors append an inline error marker. Detects mongo cursor-style streams via .close().
CSP stream methods now return Readable; routes own the piping via streamJsonArray. The HTTP framing, mid-stream error injection, and client-disconnect cleanup that were duplicated across ExternalApiStream and Storage now live in one place at the route layer. Touches all six stream entry points (address, tx, block, wallet addresses/transactions/utxos) plus the gnosis multisig route, across internal, base EVM, Moralis, MultiProvider, SVM, and Ripple. Storage's apiStreamingFind drops req/res and returns the cursor stream directly. Stream params types lose req/res; integration tests pipe the returned stream through streamJsonArray. Auto-detects a jsonl flag on the returned stream so chain-specific formatting stays in the CSP without leaking through routes.
ExternalApiStream.onStream, NodeQueryStream.onStream, Storage.apiStream, and Storage.stream all duplicated the same JSON-array framing and client-disconnect handling. With the route-layer streamJsonArray helper now owning that logic and every CSP migrated to return Readable, these are unused and removed along with their express imports.
Lets CSPs that produce newline-delimited JSON (wallet transactions, SVM streams) set stream.jsonl = true once at construction. Routes call streamJsonArray uniformly without needing chain-specific format checks. The opts.jsonl override still wins when the caller passes one.
streamJsonArray's req/res close handlers now also call stream.destroy(),
not just stream.close(). Transform pipelines like the EVM and Gnosis
wallet-tx streams have no .close(), so disconnects previously left them
running until the next data event triggered cleanup. Destroying the
transform fires its 'close' event, which the CSP layer hooks for cursor
teardown — restoring the synchronous cleanup the old req.on('close')
wiring provided.
Also documents two intentional behavioral nuances surfaced during
review: ERC-20 transfers now stream as a multi-line JSON array (same
JSON value, different whitespace from the prior res.json), and the
empty-wallet 400 now uses the JSON {error, message} body shape from
respondWithError instead of text/plain.
streamJsonArray now settles its returned promise on req/res close and on stream 'close', not just on 'end' or 'error'. Aborted requests previously left the route handler awaiting forever because tearDown destroyed the stream but never resolved the promise, and a destroyed pipeline does not always emit 'end' upstream. Single-shot guards (safeResolve/safeReject) prevent double-settlement when these races overlap. EVM streamWalletTransactions now collects cursor-cleanup callbacks via streamParams.cleanups and runs them when the FINAL piped stream closes or ends. The previous wiring attached cleanup to the intermediate transform inside _buildWalletTransactionsStream, but the caller adds further eventPipe stages on top of it; destroy() does not reliably propagate upstream through those, so a mid-stream client disconnect could leak the Mongo cursor until natural exhaustion.
Cover the JSON-array and JSONL framing paths, the inline-error and pre-data error branches, and the client-disconnect promise settlement that previously hung the route handler. Also assert the EVM wallet-tx pipeline closes its Mongo cursor when the final stream is destroyed, exercising the cleanup-array wiring through every pipe stage the route sees.
Setting application/json on a newline-delimited stream tripped up JSON-aware HTTP clients (supertest's auto-parser among them) that tried to parse the whole body as a single JSON document. Switching to application/x-ndjson in jsonl mode keeps array responses on application/json while telling clients the right thing about the shape of the body. Updates the EVM memory-leak tests to drop the obsolete req/res params on streamWalletTransactions and pipe the returned stream through streamJsonArray, matching the rest of the migrated callers.
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.
Description
CSPs were carrying
reqandresthrough the whole call stack so that piping into the response could happen deep inside the provider. The HTTP framing moves back to the route layer via a newstreamJsonArrayhelper, CSPs returnReadablestreams, and the three duplicate piping helpers (ExternalApiStream.onStream,Storage.apiStream,Storage.stream) are gone.Changelog
streamJsonArrayhelper inroutes/apiUtils.ts; supports JSON array (default) and JSONL via astream.jsonlflag the helper auto-detects, so routes stay chain-agnosticaddress,tx,block, walletaddresses/transactions/utxos) plus the Gnosis multisig route across internal, base EVM, Moralis, MultiProvider, SVM, and Ripple to return streamsonStream/apiStream/Storage.streamhelpers along with theirexpressimportsTesting Notes
npm run test:unitinbitcore-node(281 passing; 8 new tests forstreamJsonArrayand EVM cursor-cleanup)/api/<chain>/<network>/address/:addr,/tx,/block, and the wallet endpoints; output format matches the prior version/address/:addr?tokenAddress=...returns a JSON array of transfer objects (see Behavioral Notes)Behavioral notes
Two intentional output changes worth calling out for any byte-level consumer:
[\n{..},\n{..}\n]) instead ofres.json()'s compact form. Same JSON value, only inter-element whitespace differs.text/plain "No addresses found for wallet"to the JSON{error, message}shape used by every other 4xx path.Checklist
BWCif modifying the bitcore-wallet-client package,CLIif modifying the bitcore-cli package, etc.)