Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release/8.0] Handle rejected promises inside incoming Invocation messages #55230

Merged
merged 2 commits into from
May 2, 2024
Merged

[release/8.0] Handle rejected promises inside incoming Invocation messages #55230

merged 2 commits into from
May 2, 2024

Commits on Apr 19, 2024

  1. 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)
```
    @damirault
    damirault authored and github-actions committed Apr 19, 2024
    Configuration menu
    Copy the full SHA
    5f978c2 View commit details
    Browse the repository at this point in the history

  2. 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)
    @damirault
    damirault authored and github-actions committed Apr 19, 2024
    Configuration menu
    Copy the full SHA
    37f7601 View commit details
    Browse the repository at this point in the history