got icon indicating copy to clipboard operation
got copied to clipboard

got.stream documentation does not mention returning a Promise

Open meganwalker-ibm opened this issue 3 years ago • 1 comments

Describe the bug

  • Node.js version: v14.18.3
  • OS & version: MacOS Montery 12.1

got.stream seems to return an undocumented Promise in some circumstances. This is evident when you pass invalid Options that fail to be validated by got, which then triggers a Rejected Promise to be returned to the user. The Documentation only mentions that got.stream returns a Duplex.

Actual behavior

An undocumented Promise is returned and Rejected due to the incorrect options, this can cause UnhandledPromiseRejectionWarning if the user does not realise that a Promise can be returned from got.stream

(node:44188) UnhandledPromiseRejectionWarning: RequestError: Unexpected option: meaningless
    at Request._destroy (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/core/index.js:478:21)
    at Request.destroy (internal/streams/destroy.js:39:8)
    at Request.flush (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/core/index.js:249:22)
    at lastHandler (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/create.js:37:26)
    at iterateHandlers (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/create.js:49:28)
    at got (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/create.js:69:16)
    at Function.got.stream (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/create.js:169:37)
    at Options.merge (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/core/options.js:406:27)
    at new Options (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/core/options.js:344:26)
    at new Request (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/core/index.js:233:28)
    at got (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/create.js:31:25)
    at Function.got.stream (file:///Users/megan.walker/firefly/firefly-request-helper/node_modules/got/dist/source/create.js:169:37)
    at makeRequest (/Users/megan.walker/firefly/firefly-request-helper/delete-body-test.js:8:32)

...

Expected behavior

Either a - the Promise response is documented for got.stream(), making clear the Request it returns can be a Promise. b - the rejected Promise is caught by got, and is converted into calling the stream's .on('error', <...>) handler

Now we know that it returns a promise in some circumstances despite the user asking for a stream, documenting would be sufficient to resolve this; though I'm not 100% sure if that's the 'right' thing to do, when the user has asked for a stream instead. ...

Code to reproduce

const options = {
  url: 'https://httpbin.org/get',
  meaningless: 'option'
}

const makeRequest = async (options) => {
  const { default: got } = await import('got')
  const ourRequestStream = got.stream(options)
  ourRequestStream.catch(e => {
    console.log('promise error')
  })
  ourRequestStream
    .on('error', (e) => {
      console.log('stream error')
    })
}

makeRequest(options)

This prints out 'promise error'. Failing to make use of the undocumented .catch() function from the Promise means you're likely to get an UnhandledPromiseRejectionWarning unless you have code to catch those more generally. It does not print the 'stream error' line, nor do any event handlers get called.

Checklist

  • [X] I have read the documentation.
  • [X] I have tried my code with the latest version of Node.js and Got.

meganwalker-ibm avatar Jan 20 '22 11:01 meganwalker-ibm

I've observed this as well. It worked in earlier versions of Got (I think 10.x), but is currently not working without an extra await.

I had this code:

const source = client.stream('https://example.com/')

for await (const chunk of source) {
  console.log(chunk)
}

Which now results in:

TypeError: data is not async iterable

Adding await to the return value of client.stream fixes it:

const source = await client.stream('https://example.com/')

for await (const chunk of source) {
  console.log(chunk)
}

LinusU avatar Nov 08 '22 15:11 LinusU