Node-fetch: Bug: missing return after "reject(error)"

Created on 27 Jun 2020  路  4Comments  路  Source: node-fetch/node-fetch

Missing return after "reject(error)". See https://github.com/node-fetch/node-fetch/blob/0f0e6253ae92c5192b07803e4176df0f4e69f8ff/src/index.js#L124

Your Environment

| software | version
| ---------------- | -------
| node-fetch | 3.0.0-beta.7
| node | v14.4.0
| npm | 6.14.5
| Operating System | Ubuntu 18.04.4

bug

All 4 comments

Thanks for the report!

Note to contributors:\
We should probably make this an asynchronous function.

I would quite like to get rid of the top promise wrapper if maybe possible. then you can remove all edge cases where you have to call resolve, reject and stop any code to continue (like in this case) by only using a return or a throw, and also remove one long indentation

https://github.com/node-fetch/node-fetch/blob/0f0e6253ae92c5192b07803e4176df0f4e69f8ff/src/index.js#L34-L36

having a async function that returns a promise is pointless, could just as well remove the async keyword at the top, but i would rather want to make the .on('response') to a promise instead using something of a utility function:

function once(obj, event) {
  return new Promise((resolve, reject) => {
    // remove the error listener once it completes
    const data = value => {
      obj.removeListener('error', error)
      resolve(value)
    }
    // remove the event listener once it errors
    const error = err => {
      obj.removeListener(event, data)
      reject(err)
    }
    obj.once(event, data).once('error', error)
  })
}
- request_.on('response', response_ => {
+ const response_ = await util.once(request_, 'response')

Thanks for the quick response!

About the once util. Node.js already has this util. The once was added in v11.13.0, v10.16.0. And these Node.js versions are supported by node-fetch

By the way, improved error handling will solve issues like this: https://github.com/node-fetch/node-fetch/issues/889#issuecomment-651662680

889 is same, I would go directly with async function and a try...catch to be able to throw a meaningful Error. This should and must be fixed.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

runarberg picture runarberg  路  5Comments

Janpot picture Janpot  路  4Comments

caub picture caub  路  4Comments

jpierson picture jpierson  路  4Comments

teolag picture teolag  路  5Comments