Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stringify BigInts #662

Open
ntr-808 opened this issue Mar 12, 2021 · 2 comments · May be fixed by #713
Open

Stringify BigInts #662

ntr-808 opened this issue Mar 12, 2021 · 2 comments · May be fixed by #713

Comments

@ntr-808
Copy link

ntr-808 commented Mar 12, 2021

> const log = require('bunyan').createLogger({ name: "bigints" });
undefined
> log.info({ bignumber: 11111n }, 'here is a bigint');
Uncaught TypeError: Do not know how to serialize a BigInt
    at JSON.stringify (<anonymous>)
    at module.exports (/home/ntr/synth/synth-library/api-ts/node_modules/safe-json-stringify/index.js:66:14)
    at fastAndSafeJsonStringify (/home/ntr/synth/synth-library/api-ts/node_modules/bunyan/lib/bunyan.js:1220:24)
    at Logger._emit (/home/ntr/synth/synth-library/api-ts/node_modules/bunyan/lib/bunyan.js:911:15)
    at Logger.info (/home/ntr/synth/synth-library/api-ts/node_modules/bunyan/lib/bunyan.js:1045:24)
> 

I have submitted a PR to match so this issue will be fixable simply by a version upgrade.
debitoor/safe-json-stringify#12

@danielemery
Copy link

Just noting we are running into the same issue. Doesn't look like the PR is getting much (any) attention.

Does anyone have a good work around in the meantime? Is it possible to add a serializer to bunyan that can target BigInts without having to define every key that might contain a BigInt?

@amccarthy1 amccarthy1 linked a pull request Aug 4, 2023 that will close this issue
@amccarthy1
Copy link

I took a stab at fixing this using the second arg to JSON.stringify rather than an upstream change to safeJsonStringify, this has the added benefit of being able to natively work with JSON.stringify which might be a bit faster.

@trentm I know this is an old issue and it's been a while, but I was hoping you could take a look at this PR (which has unit tests and follows the contributor guidelines). This is a pretty impactful fix, since the bug results in the default serializer throwing an error. I'd really appreciate it! I can work around by patching my own version of this, but I would love for others doing similar things to get the benefit too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants