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

Error tag incorrectly set (node 11) #3898

Closed
mfanto opened this issue Jan 8, 2019 · 5 comments
Closed

Error tag incorrectly set (node 11) #3898

mfanto opened this issue Jan 8, 2019 · 5 comments
Assignees
Labels
Milestone

Comments

@mfanto
Copy link

@mfanto mfanto commented Jan 8, 2019

All requests, including with the Hello World tutorial on https://hapijs.com, show the error tag.

Are you sure this is an issue with the hapi core module or are you just looking for some help?

Yes, I believe so.

Is this a security related issue?

No

What are you trying to achieve or the steps to reproduce?

Using the following example: https://gist.github.com/mfanto/9ffac7d26a26cd1649c9ff6810be9d73

What was the result you received?

Debug: request, error, close
{ request: true, error: true, close: true }
Debug: response, error, close
{ response: true, error: true, close: true }

The error tag is set to true, both through debugging turned on, as well as the server request event.

What did you expect?

The error tag to not be set.

Context

  • node version: 11.6.0
  • hapi version: 17.8.1
  • os: Mac OSX

I think the problem is with this line:

request._log(err ? ['request', 'error'] : ['request', 'error', event], err);

Regardless if err is truthy or not, it always sets the error tag.

@devinivy
Copy link
Member

@devinivy devinivy commented Jan 8, 2019

I can confirm also on hapi v17.7.0 and node v11.2.0 with a simple curl request. I can also confirm that the issue does not occur on node v10.12.0 with the same hapi version. I recall some timer issues in node v11 that @cjihrig experienced and reviewed (i.e. nodejs/node#24322). While those bugs were fixed in v11.2.0, perhaps there's something else going on in a similar vein.

@cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jan 8, 2019

I don't think this is related to the timers issues (knocks on wood).

nodejs/node#20611 landed in Node 10.2.0. If you run the example against Node 10.2.0, you'll see the same output as you see on Node 11. That commit was determined to be a breaking change, and reverted in Node 10.8.0 in nodejs/node#21809, which is when the behavior goes back to what we expect.

nodejs/node#20611 was then re-released in Node 11, which is why we see the different behavior. I think hapi will probably want to update to deal with this.

@mfanto
Copy link
Author

@mfanto mfanto commented Jan 8, 2019

I don't know if this additional information is helpful, but it looks like when running against v10.15.0, I don't see the request event fire at all (as shown in my original gist).

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Jan 15, 2019

v8 and v10 do not have this problem, hence nothing is being reported (the internal request events are only fired on errors). With v11, looks like close is now a normal event, not just an error one.

@hueniverse hueniverse self-assigned this Jan 15, 2019
@hueniverse hueniverse added the bug label Jan 15, 2019
@hueniverse hueniverse added this to the 18.0.0 milestone Jan 15, 2019
@hueniverse hueniverse changed the title Error tag incorrectly set Error tag incorrectly set (node 11) Jan 15, 2019
@lock
Copy link

@lock lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants