Skip to content

Adapt network manager handlers to copy rather than mutate requests #8090

@kanej

Description

@kanej

The hook handlers for onRequest previously ensured that a passed in request would not be modified using a deepClone. We have remove this clone as a performance improvement - but this technically alters the ownership commitments of the onRequest hook.

So at a high level:

const myRequest = {...}
const response = await next(myRequest)
doSomething(myRequest)

Next previously commited to not mutating myRequest - so doSomething(myRequest) was safe. With the removal of clone we are mutating myRequest within next. In practice this is not currently an issue.

However, we can update the internal handlers applied in onRequest so that instead of updating in place, they return a new object.

See

const requestHandlers = await initializationMutex.exclusiveRun(
async () => {
let handlersPerConnection =
requestHandlersPerConnection.get(networkConnection);
if (handlersPerConnection === undefined) {
handlersPerConnection =
await createHandlersArray(networkConnection);
requestHandlersPerConnection.set(
networkConnection,
handlersPerConnection,
);
}
return handlersPerConnection;
},
);
// We clone the request to avoid interfering with other hook handlers that
// might be using the original request.
let request = await deepClone(jsonRpcRequest);
for (const handler of requestHandlers) {
const newRequestOrResponse = await handler.handle(request);
if (isJsonRpcResponse(newRequestOrResponse)) {
return newRequestOrResponse;
}
request = newRequestOrResponse;
}
return next(context, networkConnection, request);

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Status

    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions