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

Some tests fail on Node.js v13 #1548

Open
jonchurch opened this issue Mar 6, 2020 · 2 comments
Open

Some tests fail on Node.js v13 #1548

jonchurch opened this issue Mar 6, 2020 · 2 comments

Comments

@jonchurch
Copy link

jonchurch commented Mar 6, 2020

Just wanted to make a note that it looks like some changes in v13 of Node.js have broken some functionality here in strange ways. I noticed this when investigating kwhitley/apicache#189, where some of their tests are failing based on supertest timeouts (and not a stable set, some almost always do, and others are intermittent).

I opened this issue here instead of supertest because some of supertest's tests fail too with the same timeout errors from apicache's tests, and the tests that fail here.

I wasn't able to tell what the issues are exactly, just that they give the following error across all three repos' tests (including this one's):

Error: Timeout of 5000ms exceeded. 
For async tests and hooks, ensure "done()" is called; 
if returning a Promise, ensure it resolves.

Steps to Reproduce

nvm install 13.10.1
node -v # 13.0.1 or up to current 13.10.1

cd superagent

npm t

After poking around, I wasn't able to figure anything out conclusive, but it seems the callback to .end is never being called. In at least one test I was able to narrow it down to a change in v13 that I think could cause the behavior.

https://github.com/visionmedia/superagent/blob/0de12b299d5d5b5ec05cc43e18e853a95bffb25a/test/node/parsers.js#L69-L81

This test fails, and the .end callback is never called. I think that could be because in v13 aborted requests no longer emit the end event, which I think superagent is using internally.

Admittedly, I do not have a clue if that's the root cause, but if there are abort calls happening in the lib in response to other things, or abort is being called by the native HTTP in response other changes in Node 13, then it would make sense that these timeout errors pop up because end never has its callback invoked.

@SpineEyE
Copy link

sindresorhus/got#1107 related?

@jonchurch
Copy link
Author

jonchurch commented Mar 23, 2020

@SpineEyE The change breaking the linked issue was reverted w/ Node.js v13.11
Edit: Maybe not fully fixed yet actually, looks like got's tests are still breaking currently

The timeout error I reported here still happens on 13.11, I'm assuming due to the new behavior w/ abort which isn't planned for reversion so far as I know.

And to be clear w/ the maintainers, I know that v13 is experimental and it's expected for things to be breaking. Just want to track this for anyone experiencing it 👍

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

No branches or pull requests

2 participants