Function.prototype.toString requires that 'this' be a Function
chrisands opened this issue · 2 comments
chrisands commented
It seems that pino-pretty crashes when tries to serialize object with null prototype.
Ran into problem when tried to log request.query
from fastify
.
TypeError: Function.prototype.toString requires that 'this' be a Function
at toString (<anonymous>)
at getCleanClone (/Users/osiyuk/Projects/github/pino-pretty-issue/node_modules/.pnpm/fast-copy@3.0.1/node_modules/fast-copy/dist/cjs/index.cjs:49:27)
at copyObjectLooseModern (/Users/osiyuk/Projects/github/pino-pretty-issue/node_modules/.pnpm/fast-copy@3.0.1/node_modules/fast-copy/dist/cjs/index.cjs:213:17)
at copier (/Users/osiyuk/Projects/github/pino-pretty-issue/node_modules/.pnpm/fast-copy@3.0.1/node_modules/fast-copy/dist/cjs/index.cjs:362:20)
at copy (/Users/osiyuk/Projects/github/pino-pretty-issue/node_modules/.pnpm/fast-copy@3.0.1/node_modules/fast-copy/dist/cjs/index.cjs:375:16)
at filterLog (/Users/osiyuk/Projects/github/pino-pretty-issue/node_modules/.pnpm/pino-pretty@10.2.3/node_modules/pino-pretty/lib/utils/filter-log.js:30:19)
at Object.pretty (/Users/osiyuk/Projects/github/pino-pretty-issue/node_modules/.pnpm/pino-pretty@10.2.3/node_modules/pino-pretty/lib/pretty.js:81:11)
at Object.<anonymous> (/Users/osiyuk/Projects/github/pino-pretty-issue/index.js:18:14)
at Module._compile (node:internal/modules/cjs/loader:1256:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
Reproduction:
const pp = require('pino-pretty');
const qs = require('fast-querystring')
const pretty = pp.prettyFactory();
const nullObj = qs.parse('{}')
process.stdout.write(pretty(nullObj) + '\n');
"fast-querystring": "^1.1.2",
"pino-pretty": "^10.2.3"
jsumners commented
Ugh, I absolutely despise Object.create(null)
. Would you like to submit a PR to fix this? Remember to add unit tests.
chrisands commented
Just found time to tackle the issue. Here's what I found out.
Object.create(null)
works fine and doesn't have problem. The problem is that fast-querystring
uses V8 optimization by defining function replacing it's prototype
// Optimization: Use new Empty() instead of Object.create(null) for performance
// v8 has a better optimization for initializing functions compared to Object
const Empty = function () {};
Empty.prototype = Object.create(null);
I'm not sure if it is a good idea to go tweak fast-copy
to handle this case. @jsumners WDYT?
Notes about fast-copy
functiongetCleanClone
should handle case where var Constructor = prototype.constructor
=== undefined
that would mean that this is null prototype and we should return create(null)