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

Do not use deprecated request.abort() #1242

Merged
merged 8 commits into from May 10, 2020
Merged

Do not use deprecated request.abort() #1242

merged 8 commits into from May 10, 2020

Conversation

Giotino
Copy link
Collaborator

@Giotino Giotino commented May 10, 2020

Checklist

  • I have read the documentation.
  • I have included some tests.

Fixes #1216

@szmarczak
Copy link
Collaborator

szmarczak commented May 10, 2020

The tests are still failing sorry forgot to npm run build

@szmarczak
Copy link
Collaborator

The .aborted getter is still invalid, but lemme fix it :)

@Giotino
Copy link
Collaborator Author

Giotino commented May 10, 2020

The tests are still failing sorry forgot to npm run build

Yeah, I missed the stream test, I was fixing them.
I also added a build on TravisCI with Node.JS 14, since the related issue was version related.

@szmarczak
Copy link
Collaborator

I noticed a bug.

z = require('.').stream('https://example.com');
z.resume();
z.once('end', () => { console.log('end'); });
// If you wait for the `end` event, then console.log(z.aborted) it will give you `true`
// That's because the this[kResponse].complete is false for some reason

But that doesn't happen with the native Node.js module only:

require('https').get('https://example.com', response => { response.resume(); response
.once('end', () => { console.log(response.complete); }); })

@szmarczak
Copy link
Collaborator

Fortunately I'm about to fix it

@szmarczak
Copy link
Collaborator

Unfortunately it's harder than it seems 😅

@szmarczak
Copy link
Collaborator

At least I know that it's caused somehow by decompress-response...

@szmarczak
Copy link
Collaborator

@szmarczak
Copy link
Collaborator

Tests pass on Node.js 14, but fail on 10, 12 and 13. I'll fix this now.

@szmarczak
Copy link
Collaborator

nodejs/node#33335

@szmarczak
Copy link
Collaborator

nodejs/node#33336

@szmarczak szmarczak changed the title Node.JS request.abort() deprectated Do not use deprecated request.abort() May 10, 2020
@szmarczak szmarczak merged commit ab338a7 into sindresorhus:master May 10, 2020
@szmarczak
Copy link
Collaborator

Thanks! 🙌 Nice work.

@Giotino Giotino deleted the request-abort-fix branch May 10, 2020 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Responses are not cached on Node.js ≥ v14.0.0
2 participants