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

Inconsistent behavior between callback and promise styles in json-rpc-engine #1992

Open
mcmire opened this issue Sep 26, 2022 · 0 comments
Open

Comments

@mcmire
Copy link
Contributor

mcmire commented Sep 26, 2022

You can use engine.handle two ways:

  • If you pass a callback, then the callback will be called with two arguments: error and response. Assuming that the request is valid, the request will be passed through the middleware stack. If an error is thrown anywhere along the way, the callback will be called with the error as the first argument; otherwise it will be called with the response as the second argument. An exception is made for notifications.

  • If you don't pass a callback, then engine.handle returns a promise. Again, the request is passed through the middleware stack. If an error is thrown along the way, then it will be added to the response object. Regardless, the promise will resolve with the response object. (Again, an exception is made for notifications.)

Said another way, when an error occurs, in the callback style, error will be populated, even if res.error will also be populated; but in the promise style, the promise will always resolve (and res.error will be populated). This inconsistency is problematic.

Why? In our various projects, we very frequently make use of pify or util.promisify to convert a function that takes an errback to promise style. However, if you take engine.handle and promisify it, it will have different behavior than the promise form of engine.handle. That's surprising.

To address this, it seems like it might be beneficial to go one of two routes:

  • Change the callback form so that we set error to undefined after we set it on res.error
  • Change the promise form so that we reject when we have an error rather than resolving.
@MajorLift MajorLift transferred this issue from MetaMask/json-rpc-engine Nov 7, 2023
@MajorLift MajorLift changed the title Inconsistent behavior between callback and promise styles Inconsistent behavior between callback and promise styles in json-rpc-engine Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants