Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an unhandled promise rejection that was reported in issue #1307, where the internal state fetch inside onStateChange (when called with emitCurrentState: true) could throw a BleError that was not caught, causing Sentry and other error tracking tools to surface it as an unhandled promise rejection even when users had their own error handlers.
Changes:
- Converts the fire-and-forget
.then(successCb)call on the internalthis.state()promise to.then(successCb, rejectionCb)with an empty rejection handler, preventing the unhandled promise rejection
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| () => { | ||
| // Prevent unhandled promise rejection for internal state fetch. | ||
| } |
There was a problem hiding this comment.
There is no test covering the emitCurrentState: true path in onStateChange, and in particular no test verifying that when the internal state fetch rejects, the promise rejection is swallowed and does not propagate (the core behavior this fix adds). The existing test at line 106 only exercises the emitCurrentState: false (default) code path. Given that the rest of BleManager.js is thoroughly covered and the whole purpose of this PR is to prevent an unhandled rejection, a test ensuring this behavior works correctly (e.g. mocking BleModule.state to reject and asserting no unhandled rejection is thrown) would be valuable.
Adds a rejection handler to the internal state fetch when a listener is attached to avoid potential unhandled promise rejections.
Fixes #1315