Skip to content

Commit

Permalink
[release/8.0] Handle rejected promises inside incoming Invocation mes…
Browse files Browse the repository at this point in the history
…sages (#55230)

* Handle rejected promises inside incoming Invocation messages

Incoming messages of Invocation type result in executing a clientMethod promise, but promise rejections are unhandled.
This can result in a unhandled rejected promise, potentially resulting in Node.js process crashing.
Note, this was previously detected via eslint and suppressed in code.

To avoid this, catch promise rejections and log the failure.

One scenario this case can happen is, if during the clientMethod call, the server connection is disconnected, the invokation will still attempt to send a completion message (through _sendWithProtocol). This will then result in a promise rejection, such as the following:

```
Cannot send data if the connection is not in the 'Connected' State.
    at HttpConnection.send (node_modules\@microsoft\signalr\dist\cjs\HttpConnection.js:95:35)
    at HubConnection._sendMessage (node_modules\@microsoft\signalr\dist\cjs\HubConnection.js:266:32)
    at HubConnection._sendWithProtocol (node_modules\@microsoft\signalr\dist\cjs\HubConnection.js:273:21)
    at HubConnection._invokeClientMethod (node_modules\@microsoft\signalr\dist\cjs\HubConnection.js:577:24)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
```

* Add test of callback sending response with a rejected promise

Adds a test covering when callback invoked when server invokes a method on the client and then handles rejected promise on send

When the unhandled promise fix is not applied to HubConnection,
 the following error fails the HubConnection.test.ts test suite:

    Send error

      831 |                         connection.send = () => {
      832 |                             promiseRejected = true;
    > 833 |                             return Promise.reject(new Error("Send error"));
          |                                                   ^
      834 |                         }
      835 |                         p.resolve();
      836 |                     });

      at TestConnection.connection.send (signalr/tests/HubConnection.test.ts:833:51)
      at HubConnection.send [as _sendMessage] (signalr/src/HubConnection.ts:422:32)
      at HubConnection._sendMessage [as _sendWithProtocol] (signalr/src/HubConnection.ts:433:25)
      at HubConnection._sendWithProtocol [as _invokeClientMethod] (signalr/src/HubConnection.ts:808:24)

---------

Co-authored-by: Danny Amirault <dana@microsoft.com>
  • Loading branch information
github-actions[bot] and damirault committed May 2, 2024
1 parent c21f27d commit 12adf59
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/SignalR/clients/ts/signalr/src/HubConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -614,8 +614,10 @@ export class HubConnection {

switch (message.type) {
case MessageType.Invocation:
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this._invokeClientMethod(message);
this._invokeClientMethod(message)
.catch((e) => {
this._logger.log(LogLevel.Error, `Invoke client method threw error: ${getErrorString(e)}`)
});
break;
case MessageType.StreamItem:
case MessageType.Completion: {
Expand Down
32 changes: 32 additions & 0 deletions src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,38 @@ describe("HubConnection", () => {
});
});

it("callback invoked when server invokes a method on the client and then handles rejected promise on send", async () => {
await VerifyLogger.run(async (logger) => {
const connection = new TestConnection();
const hubConnection = createHubConnection(connection, logger);
let promiseRejected = false;
try {
await hubConnection.start();
const p = new PromiseSource<void>();
hubConnection.on("message", async () => {
// Force sending of response to error
connection.send = () => {
promiseRejected = true;
return Promise.reject(new Error("Send error"));
}
p.resolve();
});
connection.receive({
arguments: ["test"],
nonblocking: true,
target: "message",
invocationId: "0",
type: MessageType.Invocation,
});

await p;
expect(promiseRejected).toBe(true);
} finally {
await hubConnection.stop();
}
}, new RegExp("Invoke client method threw error: Error: Send error"));
});

it("stop on handshake error", async () => {
await VerifyLogger.run(async (logger) => {
const connection = new TestConnection(false);
Expand Down

0 comments on commit 12adf59

Please sign in to comment.