-
Notifications
You must be signed in to change notification settings - Fork 28.6k
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
Change JSON.stringify behaviour on certain builtin objects #22498
Comments
I don't think having a Could you specify the use case in which you are transporting an |
@jsmrcaga We used library that do something like that:
Then we just blindly passed thrown error to logger. |
Should i override |
we should have |
Why not throw an exception? If the resault is unwanted behavior, I'd call that an error, rather then ignore it. |
@refack because the objects might be in unsavory places. for example from the OP: |
I agree, we will not want to failed to whole function becouse of one (or more) unwanted instances, returning {} is a good solution. If agreed i can send a PR with a fixed for this 3 classes. |
I would prefer to know type of stringified object - eg. |
@Ginden that type of behaviour should be overseen by some bigger instance (TC39?). It changes the whole behaviour for
you would get |
@jsmrcaga Why should it? Defining custom Stringifing |
That is the Node.js way, fail on uncaught errors. Trying to serialise something that is unserializable is an error. > var a = {}
undefined
> var b = {}
undefined
> a.b = b
{}
> b.a = a
{ b: { a: [Circular] } }
> JSON.stringify(a)
TypeError: Converting circular structure to JSON
at JSON.stringify (<anonymous>)
> A solution could be to make |
I am not a big fan of controlling behavior through environment variables or command line options but they seem to be good fits to resolve the error v.s. special output decision issue. EDIT: making underscore properties non-enumerable (suggested by @refack) sounds like a good idea as well |
@refack i'm saying that this shouldn't be an error case at all, not that we should implicitly be catching some error somewhere. @joyeecheung i̛'m̡ ͢af̴r̴aid |
@devsnek I get your POV, but if the only three option are (1) keep this as this is (2) do something tricky (3) error. I prefer error, just because of the principal of least surprise. |
Currently I use following code loaded before any user code: import { builtinModules } from 'module';
function censoredToJson() {
const filteredObject = Object.fromEntries(
Object.entries(this).map(([k, v]) => {
if (k.startsWith('_')) {
return [k, null];
}
return [k, v];
}),
);
return {
$type: this.constructor.name,
...filteredObject,
};
}
for (const moduleName of builtinModules) {
const data = require(moduleName);
for (const v of Object.values(data)) {
if (typeof v === 'function' && v.prototype) {
const hasMethods = Object.getOwnPropertyNames(v.prototype).length > 1;
if (hasMethods && !('toJSON' in v.prototype)) {
console.log(`Decorating ${moduleName}.${v.name}`);
Object.defineProperty(v.prototype, 'toJSON', {
value: censoredToJson,
enumerable: false,
configurable: true,
});
}
}
}
} It didn't break anything yet. |
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document. |
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document. |
There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment. For more information on how the project manages feature requests, please consult the feature request management document. |
This is not a first time when it happens in my work.
We accidentaly called
JSON.stringify
onError
instance with certain non-standard properties. These objects contained_response
field, instance ofhttp.ServerResponse
. Stringifyng such objects results in giant JSON that can't be used in future (or at least, I can't think of common use case for stringifyinghttp.ServerResponse
).Therefore, I would suggest "blocking" stringifying instances of following classes:
WritableStream
net.Server
ReadableStream
This blocking can be implemented as throwing in
.toJSON
method, making.toJSON
return subset of object or making it return string"[Object net.Server]"
instead of exposing whole internals of an object.The text was updated successfully, but these errors were encountered: