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

POST request with ReadStream request body does not return a response #2595

Closed
1 of 2 tasks
busma13 opened this issue Feb 28, 2024 · 10 comments
Closed
1 of 2 tasks

POST request with ReadStream request body does not return a response #2595

busma13 opened this issue Feb 28, 2024 · 10 comments
Labels

Comments

@busma13
Copy link

busma13 commented Feb 28, 2024

Please avoid duplicates

Reproducible test case

https://github.com/busma13/nock-stream-test

Nock Version

13.5.4

Node Version

20.10.0

TypeScript Version

No response

What happened?

I'm using got to send a post request to an api that will upload a file. The request body contains a ReadStream of the file to be uploaded. In my jest test Nock successfully intercepts the request, the InterceptedRequestRouter class calls handleWrite(), but handleEnd() is never called. It will hang until the jest test times out.

If I switch to node version 20.9.0 or previous Nock will successfully read the stream and return a response.

Would you be interested in contributing a fix?

  • yes
@busma13 busma13 added the bug label Feb 28, 2024
@mikicho
Copy link
Contributor

mikicho commented Mar 1, 2024

@busma13 any chance the regression is in got side? from what I see, handleEnd is never get called.

@busma13
Copy link
Author

busma13 commented Mar 4, 2024

It could definitely be got not emitting end. I will look into that. Thanks @mikicho

@matt-boris
Copy link

Hey @busma13, have you gotten any further with this? We're having a similar issue on taskcluster while trying to upgrade to the latest version of node.

@busma13
Copy link
Author

busma13 commented Mar 19, 2024

@matt-boris No I haven't, we were okay with disabling the test because the function was covered well in e2e testing, so it lost priority. I wanted to search through the got issues to see if someone else had a similar problem before making my own, but I never finished that. Are you using got to make requests as well?

@matt-boris
Copy link

@busma13 yes, we're using got as well.

@stephen-willoughby
Copy link

stephen-willoughby commented Mar 26, 2024

I think this is a got issue and not a nock issue as I did a quick switch to use axios instead of got and it worked

UPDATE: this was using got v13.0.0

@stephen-willoughby
Copy link

const nock = require('nock')
const fs = require('fs')
const axios = require('axios')

const myFile = 'somefile.txt'
const host = 'http://example.com'
const path = '/some/path'
const response = { foo: 'bar' }
const options = {
  method: 'post',
  url: host + path,
  body: fs.createReadStream(myFile)
}

nock(host)
  .post(path)
  .reply(200, response)

const run = async () => {
  const { default: got } = await import('got')

  try {
    const result = await got(options)
    console.log(result.body)
  } catch (error) {
    console.error('error')
    console.error(error)
  }
  console.log('complete')
}

run()

As a minimal recreate. The oddness is that if I switch the url to https://httpbin.org/post it works, but if I switch got to axios it works too. Almost as if it's the combination of the two

@mikicho
Copy link
Contributor

mikicho commented Mar 26, 2024

Thanks for verifying this!
Can we close this one?

@busma13
Copy link
Author

busma13 commented Mar 26, 2024

Thanks @stephen-willoughby
Yeah I'll close this.

@busma13 busma13 closed this as completed Mar 26, 2024
mifi added a commit to transloadit/uppy that referenced this issue Apr 14, 2024
because nock/nock#2595
the issue seems to have been introduced in node.js 20.10
taskcluster/taskcluster#6923
@stephen-willoughby
Copy link

Any chance this could be reopened?

got aren't interested in addressing it. And it seems to specifically be around the use of both got and nock. On sindresorhus/got#2341 someone has suggested a workaround, with a patch of nock, but I've no idea of its validity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants