Conversation
| }); | ||
| callback(serializedError, { id: req.id, jsonrpc: "2.0", error: serializedError }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Callback invoked twice when success callback throws
High Severity
handleWithCallback places both engine.handle() and the success callback(null, ...) call inside the same try block. If the callback itself throws after being invoked with a successful result, the catch block catches that error and calls the callback a second time with a serialized error response. This means the callback can be invoked twice — once with a success response and once with an error — violating the standard Node.js convention that callbacks are called exactly once. The V1 provider.send avoids this by passing the callback directly to the engine.
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
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.


Motivation and Context
Jira Link:
Description
JRPC V2 Readiness
Adds the remaining V2 integration layer (provider utilities, duplex message stream, compatibility helpers) and refactors the underlying stream infrastructure to prepare for full JRPC Engine V2 adoption.
Changes
JRPC V2 Provider & Stream Utilities
providerUtils.ts— New V2 provider utilities:providerFromEngine,providerFromMiddleware, andproviderAsMiddlewarethat work withJRPCEngineV2. Unlike V1, the V2 engine throws errors directly rather than wrapping them in response objects.messageStream.ts— NewcreateEngineStreamV2that creates a Duplex object stream backed by aJRPCEngineV2. Decouples notification forwarding from the engine via an optionalSafeEventEmitter, so the engine no longer needs to be an EventEmitter.compatibility-utils.ts— AddedpropagateToMutableRequestto deep-clone frozen request objects and propagate context properties onto the mutable clone, avoiding mutation errors in middleware chains.v2/index.ts— Exported all new V2 utilities (createEngineStreamV2,providerFromEngineV2,providerFromMiddlewareV2,providerAsMiddlewareV2, compatibility helpers).Stream Layer Refactoring
BasePostMessageStream— Refactored into an abstract class with proper encapsulation (privatefields), removedBRKprotocol, added pluggable logging via_setLogger, and made_postMessageabstract for subclass implementation.PostMessageStream— Moved message handling, constructor logic, and_destroyout of the base class; usesMessageEventprototype getters for secure origin/source validation.ObjectMultiplex— Replacedpump/end-of-streamwithpipeline/finishedfromreadable-stream; added stream state validation (destroyed/ended checks); removedwindow.consolein favor ofconsolefor non-browser compatibility.Substream— Properly typed_parentasObjectMultiplex(removedany); acceptsDuplexOptionsfor pass-through stream configuration.Stack Upgrades & Cleanup
buffer,bn.js,pump,end-of-streamdependencies in favor of native/lighter alternatives.starkeymodule andbrowserStorageutilities.@toruslabs/torus-scriptsv8,@toruslabs/configv4,@toruslabs/eslint-config-typescriptv5,@types/nodev25.Buffer.from()usages with@toruslabs/metadata-helpers(utf8ToBytes,toBufferLike).Tests
providerUtils.test.ts— Comprehensive tests forproviderFromEngine,providerFromMiddleware, andproviderAsMiddlewarecovering happy paths, error propagation,send/sendAsync/requestinterfaces, context forwarding, and round-trip composition.messageStream.test.ts— Tests forcreateEngineStreamV2covering request/notification handling, error serialization, stream push behavior, and notification emitter lifecycle.compatibility-utils.test.ts— Added tests forpropagateToMutableRequestverifying immutability of the original request, deep mutability of the clone, and correct context propagation.starkey.test.ts.How has this been tested?
Screenshots (if appropriate):
Types of changes
Checklist:
Note
Medium Risk
Medium risk because it introduces new JRPC v2 bridging primitives (
providerUtils,createEngineStreamV2) and changes how context is propagated onto requests (via deep-cloned mutable copies), which could affect consumers that adopt these exports.Overview
Completes the JRPC v2 integration surface by adding
providerUtils(providerFromEngine,providerFromMiddleware,providerAsMiddleware) and a newcreateEngineStreamV2Duplex stream that wrapsJRPCEngineV2and optionally forwards notifications via a separateSafeEventEmitter.Improves legacy-compat context propagation with
propagateToMutableRequest, which deep-clones (including nestedparams) before applying context keys to avoid mutating/failing on frozen request objects, and re-exports these new helpers fromsrc/jrpc/v2/index.ts.Adds new Vitest coverage for the stream and provider adapters and extends
compatibility-utilstests for the new mutable-propagation behavior;package-lock.jsonalso drops a number of `Written by Cursor Bugbot for commit b01101b. This will update automatically on new commits. Configure here.