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.

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

Fix stray promises when a node returns invalid results #4126

Closed
wants to merge 1 commit into from
Closed

Fix stray promises when a node returns invalid results #4126

wants to merge 1 commit into from

Conversation

marianogoldman
Copy link

This is a follow-up of #4118, as there was a second case in my original PR that was missed in the fix.

@marianogoldman
Copy link
Author

I would also like to point out that, makeError can also throw an exception and interrupt the flow of the batch processing, although this time the catch block will still reject all the promises. So, in that sense, it's still safe, but the end result could be improved if that didn't throw exception.

  const error = makeError("missing response for request", "BAD_DATA", {
      value: result, info: { payload }
  });
  this.emit("error", error);

@ricmoo
Copy link
Member

ricmoo commented Jun 7, 2023

The only time makeError should throw an error is if you pass in bad keys, which this doesn’t, no?

@ricmoo
Copy link
Member

ricmoo commented Jun 8, 2023

Ah! Yes. I'll fix this one shortly too.

@ricmoo ricmoo added bug Verified to be an issue. on-deck This Enhancement or Bug is currently being worked on. v6 Issues regarding v6 labels Jun 8, 2023
@ricmoo
Copy link
Member

ricmoo commented Jun 8, 2023

Fixed in v6.5.1.

Thanks! :)

@ricmoo ricmoo closed this Jun 8, 2023
@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Jun 8, 2023
Woodpile37 pushed a commit to Woodpile37/ethers.js that referenced this pull request Jan 14, 2024
Woodpile37 pushed a commit to Woodpile37/ethers.js that referenced this pull request Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published. v6 Issues regarding v6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants