search

ladjs / superagent

Ajax for Node.js and browsers (JS HTTP client). Maintained for @forwardemail, @ladjs, @spamscanner, @breejs, @cabinjs, and @lassjs.
https://ladjs.github.io/superagent/
MIT License
16.59k stars 1.33k forks source link

TypeError: Cannot read property '_headerSent' of undefined #1552

Open mauritsderuiter95 opened 4 years ago

mauritsderuiter95 commented 4 years ago

I'm scraping a lot of urls in search of their RSS-feeds. Some errors just quit the whole script when encountering some domains, which is of course not desired behaviour. I tracked down one of the errors.

On line 967 of node/index.js is this: if (method !== 'HEAD' && !req._headerSent) {

If I try to get "https://gtex.nl", the nodejs script exits with "TypeError: Cannot read property '_headerSent' of undefined". I know the url isn't working in browsers as well, but I think superagent has to throw an error, not stop the script entirely. So I wrote this above the current line 967:

if (!req) { console.log('Failed on url:', this.url) return this.callback(new Error('Req object doesn\'t exist')); }

Now it doesn't exit the script and just throws an error I can catch. I don't know if this is a desired solution, but can this be fixed?

Edit: To avoid confusion, I'm not the owner of gtex.nl, so I can't change whatever they did wrong.

niftylettuce commented 4 years ago

If you can provide a reproducible test case let me know.

yocontra commented 4 years ago

Just wanted to chime in that I'm also seeing this randomly - if I can come up with a reproducible case I will post back. FWIW it is happening especially when superagent-cache-plugin is used.

niftylettuce commented 4 years ago

Thank you @contra for letting me know. Would love to see a reproducible test case with express/superagent or something.

niftylettuce commented 4 years ago

The culprit seems to be this on line 526 of src/node/index.js:

delete this.req;
niftylettuce commented 4 years ago

Which in turn happens when redirection occurs.

niftylettuce commented 4 years ago

And this may be related to a redirection occurring, then having been aborted. I'm not 100%. Hopefully either of you could find the issue, I'm super limited on time.

niftylettuce commented 4 years ago

This was initially added here https://github.com/visionmedia/superagent/blame/v4.1.0/lib/node/index.js#L505 from https://github.com/visionmedia/superagent/commit/0ecc2e4f0c223375a45cb0ed8a0ad2f09be2fafb

niftylettuce commented 4 years ago

Deleting that line causes the tests to break for redirection. I think that with the information I provided above, you could write a test that reproduces this @contra ?

yocontra commented 4 years ago

@niftylettuce Yep thanks for tracking that down - I will try to debug further and hopefully submit a PR. In the meantime can you open this ticket and assign it to me so it shows up on my work list?

Weird thing in our situation is there should be no redirects, they are requests fetching off a github CDN URL that should be constant.