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
move ECONNRESET log from error to debug #1363
Conversation
Can't run tests for some reason. But for this change i don't think it's needed. |
@RTVV tests are not being run since linting is failing. You will need to fix these.
|
fastify.js
Outdated
if(err.code == 'ECONNRESET') { | ||
// chrome browser keep-alive socket connection | ||
// so after 2 min of any https request server socket will timeout | ||
log.debug({ err }, 'server socket timeout') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should just be debug
for everything.
Ok, i've moved client error to debug. What is the next step? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I think we can just land this.
@mcollina port to 1.x ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@cemremengu yes please! |
* move ECONNRESET log from error to debug * move client errors to debug
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
I'm new to js, github and this is my first PR, so feel fre to change anything.
Closes #1361
Problem
Google chrome browser keep-alive socket connection to server, so after 2 min of any https request, or if client will close browser window - server socket will timeout.
After that error will be logged:
Proposal
Errors should be something important and this one should be moved in less important category. As @mcollina suggested in previos conversation - into debug.
Checklist
npm run test
andnpm run benchmark