Backport 42 to v2.x.x

[davidmarkclements]

cc @benib

[jsumners]

Is it not possible to use the pino-std-serializers package here?

[mcollina]

I would prefer if it uses pino-std-serializers as well.

[davidmarkclements]

Good point will do tomorrow

[benib]

Ah, there is now a package for the wrapping 👍

@davidmarkclements do you have time to apply this to 4.x.x as well? Would make sense to keep the implementation in 2.x.x and 4.x.x as close as possible imho.

[davidmarkclements]

so here's the thing, we've gotten ourselves into a bit of a conflated mess here with regards to pino-std-serializers and hapi-pino – at some point there's been a sort of co-evolution and concepts have been shared between them which has lead to a corruption of terms

Specifically I'm referring to this:

https://github.com/pinojs/hapi-pino/pull/51/files#diff-168726dbe96b3ce427e7fedce31bb0bcR208

vs

https://github.com/pinojs/pino-std-serializers/blob/master/lib/req.js#L64

This makes req.raw in hapi-pino significantly different to req.raw in pino-std-serializers.

In Hapi the request object has a raw property, by default. This is an object with the shape {req, res}. While the req and res objects on request.raw do contain many (if not all) properties common to Node core req and res objects, they are in fact different objects with additional properties.

In pino-std-serializers the raw property will be the core http req object (albeit potentially decorated in the express case).

This divergence has been going on for a while, so one way for me to use pino-std-serializers is to patch https://github.com/pinojs/pino-std-serializers/blob/master/lib/req.js#L64 to say:

  _req.raw = req.raw || req

This will allow me to integrate, however it still leaves us in this situation where raw means very different things in hapi-pino vs the rest of the ecosystem which kind of violates the principle of least surprise. But maybe that's okay, because Hapi tends to do it's own thing and have it's own meaning to terms anyway.

@jsumners @mcollina – thoughts?

[jsumners]

I did ask if it is possible to use it not that it must be used.

I think I could live with:

1. the patch being

// Accommodate frameworks that attach their own `raw` object
_req.raw = req.raw || req

2. there is a note added to pino-std-serializers documentation for this method that alerts people to the potential wtf

[mcollina]

I'd be happy with option 1.

[davidmarkclements]

ok... phew.

• patched pino-std-serializers
• updated pino
• now using pino-std-serializers

I can "frontport" once we we're happy with this PR

[coveralls]

Coverage increased (+0.05%) to 99.065% when pulling b5dc054 on backport-42-to-v2.x.x into f868904 on v2.x.x.

[jsumners]

LGTM

[mcollina]

LGTM

[mcollina]

we'll need to also update the 4 branch.