-
Notifications
You must be signed in to change notification settings - Fork 13
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.
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
Improve error handling #307
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #307 +/- ##
==========================================
- Coverage 99.27% 98.32% -0.96%
==========================================
Files 11 11
Lines 416 418 +2
Branches 64 70 +6
==========================================
- Hits 413 411 -2
Misses 3 3
- Partials 0 4 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Surprisingly, this doesn't break any tests! Probably means we need to add testing for this :-) |
906985a
to
2db24ff
Compare
const status = this.pendingRequest[id as string]; | ||
if (status) { | ||
delete this.pendingRequest[id as string]; | ||
if (error === undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the key change. We may have an error response, which per the spec should result in an error response. The prior code block only checked to see if we were expecting the response with a particular ID, and early-returned. The processResult
method would still reject the promise, however it would not return an error object.
Fixes #306