-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat: Log when error occurs while pushing #28
Conversation
And upgrade `h2-auto-push` to v0.4.0. Also update other packages.
Codecov Report
@@ Coverage Diff @@
## master #28 +/- ##
==========================================
- Coverage 97.29% 94.87% -2.43%
==========================================
Files 1 1
Lines 37 39 +2
Branches 8 8
==========================================
+ Hits 36 37 +1
- Misses 1 2 +1
Continue to review full report at Codecov.
|
/cc @mcollina |
ts/src/index.ts
Outdated
@@ -70,7 +70,10 @@ async function staticServeFn( | |||
'set-cookie', | |||
cookie.serialize(CACHE_COOKIE_KEY, newCacheCookie, {path: '/'})); | |||
|
|||
pushFn(reqStream).then(noop, noop); | |||
reqStream.on('pushError', (err) => { | |||
app.log.error('Error while pushing', err); |
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.
You can use req.log.error
, in this way the log will also have the request id and support the custom log level on plugin/route basis :)
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 wanted to, but here in the onRequest
hook, the request object is not a fastify request but a raw request. Do I have a better option?
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.
log
is present in both fastify request and raw request :)
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.
Ah, good to know. Thanks. Updated.
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, once the req.log
question is figured out.
And upgrade
h2-auto-push
to v0.4.0.Also update other packages.