hapijs / hapi-pino

🌲 Hapi plugin for the Pino logger
MIT License
148 stars 61 forks source link

`req` is no longer in response event, even if `getChildBindings: (request) => { req: request }` is provided #161

Closed acofer closed 2 years ago

acofer commented 2 years ago

https://github.com/pinojs/hapi-pino/commit/42b3df4dc81073ced07161fcda9dbf12acf39a33?ts=2#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346L179

Because request.logger already exists in the if clause just above that, the logger.child() will never get called with the binding that included req, so the response event will never have req, and thus there is no way to inspect it in the logs (we have audit headers on the request that are merged into the logs in the stream pipeline we give pino).

Our current work-around is to check request in hapi onPreResponse event, and delete request["logger"] if it exists, but that feels gross.

mcollina commented 2 years ago

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

acofer commented 2 years ago

Yeah, I can give it a shot tomorrow.

acofer commented 2 years ago

Ah, sorry. That's not the bug, req is there. The problem is that we are adding the req.header that we need after the server onRequest event had already attached request.logger with the binding to {req: Request}, which did not have the header yet. I'll see if I can find the proper way/place to do it.

acofer commented 2 years ago

Figured it out. In our onPreHander event code, I did this:

    request.headers[AUDIT_HEADER_NAME] = JSON.stringify(auditData);
    // hapi-pino has already attached a logger with request bound into it without that header, so re-bind it:
    if (request.logger) {
        request.logger.setBindings({
            req: request,
        });
    }