fix: avoid override mistake

[erights]

The assignment at this location was to a self object that inherits from the console (or what was the console before this code ran). This assignment was use to override properties typically inherited from console such as error,warn,info,debug,trace. However, if the console object it inherits from is frozen, as it is in the ses-shim of HardenedJS, then this assignment fails due to the so-called "override mistake".

Changing this assignment to use Object.defineProperty instead avoids hitting the override mistake. The additional attributes I include in this change

    writable: true,
    enumerable: true,
    configurable: true,

are only there to preserve equivalence with assignment. But feel free to change them to whatever you'd like. Such changes would not affect the purpose of this PR.

[mcollina]

Can you add a test?

[erights]

Done. The added test/browser-early-console-freeze.test.js succeeds with this PR. Without this PR it fails as follows:

test/browser-early-console-freeze.test.js 2> /Users/markmiller/src/ongithub/erights/pino/browser.js:9
test/browser-early-console-freeze.test.js 2> 
self[level]=levelToValue(self.level,rootLogger)>levelToValue(level,rootLogger)?(cov_qeewo7tvl().b[37][0]++,noop):(cov_qeewo7tvl().b[37][1]++,rootLogger[baseLogFunctionSymbol][level]);cov_qeewo7tvl().s[117]++;if(
(cov_qeewo7tvl().b[39][0]++,!opts.transmit)&&(cov_qeewo7tvl().b[39][1]++,self[level]===noop)){cov_qeewo7tvl().b[38][0]++;cov_qeewo7tvl().s[118]++;return;}else{cov_qeewo7tvl().b[38][1]++;}// make sure the log
format is correct
test/browser-early-console-freeze.test.js 2>            ^
test/browser-early-console-freeze.test.js 2> 
test/browser-early-console-freeze.test.js 2> TypeError: Cannot assign to read only property 'error' of object '#<Object>'
test/browser-early-console-freeze.test.js 2>     at set (/Users/markmiller/src/ongithub/erights/pino/browser.js:9:12)
test/browser-early-console-freeze.test.js 2>     at console.setLevel (/Users/markmiller/src/ongithub/erights/pino/browser.js:3:1735)
test/browser-early-console-freeze.test.js 2>     at pino (/Users/markmiller/src/ongithub/erights/pino/browser.js:3:483)
test/browser-early-console-freeze.test.js 2>     at Test.<anonymous> (/Users/markmiller/src/ongithub/erights/pino/test/browser-early-console-freeze.test.js:7:3)
test/browser-early-console-freeze.test.js 2>     at Test.run (/Users/markmiller/src/ongithub/erights/pino/node_modules/tape/lib/test.js:127:28)
test/browser-early-console-freeze.test.js 2>     at Immediate.next (/Users/markmiller/src/ongithub/erights/pino/node_modules/tape/lib/results.js:156:7)
test/browser-early-console-freeze.test.js 2>     at process.processImmediate (node:internal/timers:476:21)
test/browser-early-console-freeze.test.js 2> 
test/browser-early-console-freeze.test.js 2> Node.js v18.19.0
​ FAIL ​ TAP
 ✖ test/browser-early-console-freeze.test.js

[erights]

The

TypeError: Cannot assign to read only property 'error' of object

is exactly the symptom we saw in our own system that led us here.

[kriskowal]

@mcollina Thanks for considering this change.

[mcollina]

lgtm

[kriskowal]

There’s a test failure affecting only Windows. Can I trouble a maintainer to re-run that to verify whether this is a flake?

[jsumners]

It's the transport tests. I wish I knew why they consistently fail as often as they do. But we can just ignore it for this.

[erights]

Thanks!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.