Fix req object placement #322

stychu · 2024-01-19T13:55:19Z

stychu · 2024-01-19T13:59:49Z

test/test.js

+    customReceivedObject: function (req) {
+      return { req }
+    },


I am bit skeptical about this test. Not sure if this change is ok or I rather should update the source code to include req at this line https://github.com/pinojs/pino-http/blob/master/logger.js#L191 as const receivedObjectResult = onRequestReceivedObject !== undefined ? onRequestReceivedObject(req, res, undefined) : { [reqKey]: req } and remove the addition in the test?

mcollina

Unfortunately creating a child logger with the given request set is critical to keep attributing all logs to the given request.

stychu · 2024-01-23T13:58:58Z

Unfortunately creating a child logger with the given request set is critical to keep attributing all logs to the given request.

Not sure if I follow. I believe it does keep the request set to all logs having it this way. req is present for all logs unless this condition is meet

pino-http/logger.js

Line 191 in 77191ae

    
           const receivedObjectResult = onRequestReceivedObject !== undefined ? onRequestReceivedObject(req, res, undefined) : {}

that why Im not sure about that one test. If the req should be added to this false evaluated condition then all logs will contain req/res objects

mcollina · 2024-02-01T09:26:11Z

I've run the example.js file in both master and this PR. It's different.

master

➜  pino-http git:(master) node example.js
{"level":30,"time":1706779462620,"pid":27894,"hostname":"mcl","req":{"id":1,"method":"GET","url":"/","headers":{"host":"localhost:3000","user-agent":"curl/8.4.0","accept":"*/*"},"remoteAddress":"::1","remotePort":56915},"msg":"something else"}
{"level":30,"time":1706779462624,"pid":27894,"hostname":"mcl","req":{"id":1,"method":"GET","url":"/","headers":{"host":"localhost:3000","user-agent":"curl/8.4.0","accept":"*/*"},"remoteAddress":"::1","remotePort":56915},"res":{"statusCode":200,"headers":{}},"responseTime":4,"msg":"request completed"}

This PR:

➜  pino-http git:(fix-nexted-key) node example.js
{"level":30,"time":1706779498790,"pid":28809,"hostname":"mcl","msg":"something else"}
{"level":30,"time":1706779498793,"pid":28809,"hostname":"mcl","req":{"id":1,"method":"GET","url":"/","headers":{"host":"localhost:3000","user-agent":"curl/8.4.0","accept":"*/*"},"remoteAddress":"::1","remotePort":56932},"res":{"statusCode":200,"headers":{}},"responseTime":3,"msg":"request completed"}

Fix req object placement

13f280a

stychu commented Jan 19, 2024

View reviewed changes

Fix typos

4faadb2

mcollina requested changes Jan 19, 2024

View reviewed changes

mcollina closed this Feb 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix req object placement #322

Fix req object placement #322

stychu commented Jan 19, 2024

stychu Jan 19, 2024 •

edited

mcollina left a comment

stychu commented Jan 23, 2024

mcollina commented Feb 1, 2024

Fix req object placement #322

Fix req object placement #322

Conversation

stychu commented Jan 19, 2024

stychu Jan 19, 2024 • edited

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

stychu commented Jan 23, 2024

mcollina commented Feb 1, 2024

stychu Jan 19, 2024 •

edited