apply serializers to args once before asObject or transmit #1971

emmyakin · 2024-05-12T23:18:10Z

Fixes serializers are applied twice in pino/browser #1970
- applySerializers functioin mutates args to logger.*, this cause args to be updated twice in asObject and transmit
- Apply serializers once before asObject
Also includes fix for browser.formatters.level does not inline attributes into message anymore #1966
- Current tests were already failing on the main branch, since formatters.level does not spread the returned object, but updates logData.level = { level: 30, label: 'info' }

emmyakin · 2024-05-12T23:19:12Z

browser.js

  const argsCloned = args.slice()
  let msg = argsCloned[0]
  const logObject = {}
  if (ts) {
    logObject.time = ts
  }
-  logObject.level = levelFormatter(level, logger.levels.values[level])
+
+  if (levelFormatter) {


restore previous implementation to fix this bug

emmyakin · 2024-05-12T23:19:39Z

browser.js

@@ -361,9 +366,9 @@ function applySerializers (args, serialize, serializers, stdErrSerialize) {
  for (const i in args) {
    if (stdErrSerialize && args[i] instanceof Error) {
      args[i] = pino.stdSerializers.err(args[i])
-    } else if (typeof args[i] === 'object' && !Array.isArray(args[i])) {
+    } else if (typeof args[i] === 'object' && !Array.isArray(args[i]) && serialize) {


no need to start a for loop, if serialize is not true

emmyakin · 2024-05-12T23:20:01Z

docs/browser.md

@@ -144,7 +144,7 @@ Unlike server pino the serializers apply to every object passed to the logger me
 if the `asObject` option is `true`, this results in the serializers applying to the
 first object (as in server pino).

-For more info on serializers see https://github.com/pinojs/pino/blob/master/docs/api.md#parameters.
+For more info on serializers see https://github.com/pinojs/pino/blob/master/docs/api.md#mergingobject.


fix docs id references

emmyakin · 2024-05-12T23:20:28Z

test/browser-serializers.test.js

@@ -284,7 +284,6 @@ test('children inherit parent serializers', ({ end, is }) => {

 test('children serializers get called', ({ end, is }) => {
  const parent = pino({
-    test: 'this',


this is not part of the pino config, or log parameters, seems like a mistake

emmyakin · 2024-05-12T23:20:54Z

test/browser.test.js

@@ -166,7 +166,7 @@ test('opts.browser.asObject logs pino-like object to console', ({ end, ok, is })
  end()
 })

-test('opts.browser.formatters logs pino-like object to console', ({ end, ok, is }) => {
+test('opts.browser.formatters (level) logs pino-like object to console', ({ end, ok, is }) => {


test names were the same, adding context for the different formatters config

emmyakin · 2024-05-12T23:24:02Z

test/browser.test.js


  end()
 })

-test('opts.browser.write func string joining when asObject is true', ({ end, ok, is }) => {


this test was repeated, see line 262 below, I deleted this, git somehow is reconciling my changes adding new test using the these lines

emmyakin · 2024-05-12T23:25:41Z

browser.js

@@ -302,7 +302,7 @@ function createWrap (self, opts, rootLogger, level) {
      const proto = (Object.getPrototypeOf && Object.getPrototypeOf(this) === _console) ? _console : this
      for (var i = 0; i < args.length; i++) args[i] = arguments[i]

-      if (opts.serialize && !opts.asObject) {
+      if (opts.serialize && !opts.transmit) {


this is the required change to fix the bug

mcollina

lgtm

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [pino](https://getpino.io) ([source](https://togithub.com/pinojs/pino)) | [`^8.18.0` -> `^9.0.0`](https://renovatebot.com/diffs/npm/pino/8.21.0/9.1.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/pino/9.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/pino/9.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/pino/8.21.0/9.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/pino/8.21.0/9.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>pinojs/pino (pino)</summary> ### [`v9.1.0`](https://togithub.com/pinojs/pino/releases/tag/v9.1.0) [Compare Source](https://togithub.com/pinojs/pino/compare/v9.0.0...v9.1.0) #### What's Changed - fix(transport-stream): Fix import error when using pkg with node v20 by [@nagyszabi](https://togithub.com/nagyszabi) in [pinojs/pino#1949 - Update LTS doc by [@jsumners](https://togithub.com/jsumners) in [pinojs/pino#1955 - Update pino types for browser.formatters by [@KatelynKim](https://togithub.com/KatelynKim) in [pinojs/pino#1946 - add node v22 to CI by [@mcollina](https://togithub.com/mcollina) in [pinojs/pino#1953 - Add Platformatic to sponsors by [@mcollina](https://togithub.com/mcollina) in [pinojs/pino#1956 - Update h3 example by [@mcollina](https://togithub.com/mcollina) in [pinojs/pino#1968 - Support file URLs when configuring multiple transports by [@haines](https://togithub.com/haines) in [pinojs/pino#1961 - Adding support for mix\&match pipelines by [@dbacarel](https://togithub.com/dbacarel) in [pinojs/pino#1954 - apply serializers to args once before asObject or transmit by [@emmyakin](https://togithub.com/emmyakin) in [pinojs/pino#1971 - build(deps-dev): bump pino-pretty from 10.3.1 to 11.0.0 by [@dependabot](https://togithub.com/dependabot) in [pinojs/pino#1933 - build(deps): bump sonic-boom from 3.8.1 to 4.0.1 by [@dependabot](https://togithub.com/dependabot) in [pinojs/pino#1960 - build(deps): bump pino-std-serializers from 6.2.2 to 7.0.0 by [@dependabot](https://togithub.com/dependabot) in [pinojs/pino#1957 - build(deps-dev): bump tsd from 0.30.7 to 0.31.0 by [@dependabot](https://togithub.com/dependabot) in [pinojs/pino#1937 - build(deps): bump actions/dependency-review-action from 3 to 4 by [@dependabot](https://togithub.com/dependabot) in [pinojs/pino#1894 - build(deps): bump pnpm/action-setup from 2.4.0 to 3.0.0 by [@dependabot](https://togithub.com/dependabot) in [pinojs/pino#1913 - Drop yarn support and update pnpm by [@mcollina](https://togithub.com/mcollina) in [pinojs/pino#1972 - build(deps): bump thread-stream from 2.7.0 to 3.0.0 by [@dependabot](https://togithub.com/dependabot) in [pinojs/pino#1959 #### New Contributors - [@nagyszabi](https://togithub.com/nagyszabi) made their first contribution in [pinojs/pino#1949 - [@haines](https://togithub.com/haines) made their first contribution in [pinojs/pino#1961 - [@emmyakin](https://togithub.com/emmyakin) made their first contribution in [pinojs/pino#1971 **Full Changelog**: pinojs/pino@v9.0.0...v9.1.0 ### [`v9.0.0`](https://togithub.com/pinojs/pino/compare/v8.21.0...4f8ea32aa69ec94b2fb5561716a7701aec991ce7) [Compare Source](https://togithub.com/pinojs/pino/compare/v8.21.0...v9.0.0) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/rustymotors/server).

apply serializers to args once before asObject or transmit

ce1e666

emmyakin commented May 12, 2024

View reviewed changes

emmyakin mentioned this pull request May 12, 2024

serializers are applied twice in pino/browser #1970

Closed

emmyakin commented May 12, 2024

View reviewed changes

mcollina approved these changes May 13, 2024

View reviewed changes

mcollina merged commit 2dda26c into pinojs:main May 13, 2024
2 checks passed

This was referenced May 15, 2024

use boolean flag to ensure serializers are applied once #1976

Merged

browser.formatters.level does not inline attributes into message anymore #1966

Closed

timtrense-leadec mentioned this pull request May 15, 2024

fix nesting of level when applying a level formatter #1967

Closed

kazimanzurrashid mentioned this pull request Jun 3, 2024

[Snyk] Upgrade pino from 9.0.0 to 9.1.0 kazimanzurrashid/file-server#442

Closed

joetitra mentioned this pull request Jun 3, 2024

[Snyk] Upgrade pino from 9.0.0 to 9.1.0 joetitra/microservices-demo#2

Open

JoKacar mentioned this pull request Jun 5, 2024

[Snyk] Upgrade pino from 8.16.1 to 9.1.0 JoKacar/opentelemetry-demo#44

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

apply serializers to args once before asObject or transmit #1971

apply serializers to args once before asObject or transmit #1971

emmyakin commented May 12, 2024

emmyakin May 12, 2024

emmyakin May 12, 2024

emmyakin May 12, 2024

emmyakin May 12, 2024

emmyakin May 12, 2024

emmyakin May 12, 2024

emmyakin May 12, 2024

mcollina left a comment

apply serializers to args once before asObject or transmit #1971

apply serializers to args once before asObject or transmit #1971

Conversation

emmyakin commented May 12, 2024

emmyakin May 12, 2024

Choose a reason for hiding this comment

emmyakin May 12, 2024

Choose a reason for hiding this comment

emmyakin May 12, 2024

Choose a reason for hiding this comment

emmyakin May 12, 2024

Choose a reason for hiding this comment

emmyakin May 12, 2024

Choose a reason for hiding this comment

emmyakin May 12, 2024

Choose a reason for hiding this comment

emmyakin May 12, 2024

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment