-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
[3.x] Fix promise rejection throwing several times #13010
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nachocodoner
changed the title
322 fix promise rejection several (wip)
322 Fix promise rejection throwing several times
Feb 19, 2024
# Conflicts: # packages/ddp-client/client/queueStubsHelpers.js
✅ Deploy Preview for v3-meteor-api-docs canceled.
|
nachocodoner
changed the title
322 Fix promise rejection throwing several times
[3.x] Fix promise rejection throwing several times
Mar 21, 2024
denihs
approved these changes
Mar 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
OSS-322
Context: #12950
I have given a reattempt to this issue. And after studying more the code and the experience so far about it I came up with the proper fix.
Within Meteor 3.x code, there are three promises utilized when invoking methods:
promise
for server promises,stubPromise
for client simulation, andserverPromise
, which acts as a wrapper forpromise
.Refer here to understand further 2.x and 3.x async behaviors.
In the Meteor 3.x codebase, we encounter several layers where promises are nested. In JavaScript, when promises are nested, unhandled exceptions from the "child" promises propagate upwards to the parent or root, resulting in uncaught errors. I've identified these instances and provided solutions for them.
Reproduction
Described a method that always throw an error on client and server side.
Then on the client side I run
callAsync
it and manage the promisesI have seen other scenarios where even more uncaught promises are showing as described on this issue, #12950
Fix
promise + stubPromise + serverPromise
With the changes on this PR.
promise
promise + stubPromise
promise + serverPromise
stubPromise
serverPromise
serverPromise + stubPromise
no promise caught