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

Error.isError necessary for properly serializing in debug tools #2

Closed
tolmasky opened this issue Feb 28, 2016 · 10 comments
Closed

Error.isError necessary for properly serializing in debug tools #2

tolmasky opened this issue Feb 28, 2016 · 10 comments
Assignees

Comments

@tolmasky
Copy link

In Tonic we serialize objects and send them over the wire to be rendered on the client. Unfortunately, Errors are the only objects we can't properly handle when creating in a different realm. I've put together this small test case:

https://tonicdev.com/tolmasky/errors-realm

As you can see, the bottom error renders "correctly": the UI knows its an error and gives it special treatment. However, in the top case it can't know it is an error, and thus defaults to object look.

@benjamingr
Copy link

We've had this problem in bluebird too where we detect non-errors for debugging purposes.

@ljharb
Copy link
Member

ljharb commented Feb 28, 2016

Thanks for contributing this use case! @benjamingr if you have concrete examples, that would help a ton :-) (filed as a separate issue or attached onto this one)

@domenic
Copy link
Member

domenic commented Mar 1, 2016

This shows nothing about the necessity of Error.isError. You would have better code if you tested e.g. Object.prototype.toString.call(obj) === "[error Error]". Then objects that wanted to opt in to error-like serialization could do so by setting their toStringTag appropriately. Or, since you are inside a single source realm, just use instanceof.

The only way Error.isError is necessary is if there is a use case for unforgably checking if something is generated from an Error object, which is not shown by this example at all. I claim there can be no such example in existence, since Error objects have no special behavior that cannot be produced by plain JavaScript objects.

@benjamingr
Copy link

@domenic worth mentioning that you can also override Symbol.hasInstance and keep using instanceof across realms.

@ljharb petkaantonov/bluebird#990 for example - there are a few others.

@tolmasky
Copy link
Author

tolmasky commented Mar 1, 2016

The purpose of the object viewer is to view the true state of the object graph in question, as such we do not want any opt-in behavior, and do in fact want the unforgable behavior. The viewer is meant to tell you whether this is in fact an Error object per the JavaScript spec. If the spec were to be changed to not have Error objects at all then perhaps that would be another solution to this issue (at which point we show them as object ALWAYS), but today as a debug tool there exists information that cannot be properly represented.

I'm not sure what you mean by being inside a single source realm as I created it in a contextify sandbox (but perhaps I simply misunderstood), this is what happens with instanceof: https://tonicdev.com/tolmasky/56d5d383c14e920d0064d060

More importantly though, instanceof can't be used since due to the Symbol.hasInstnace, instanceof may have side effects. It is absolutely crucial that during object serialization we do not affect the state of your running program.

@ljharb
Copy link
Member

ljharb commented Mar 1, 2016

There's also that despite "stack" not being part of the ES spec, native Error objects are the only objects that have a reliable stack property - that's something that's useful to have unforgeable. @benjamingr, Symbol.hasInstance only works if there's a cross-realm way to identify Error objects (and Object#toString is forgeable due to Symbol.toStringTag)

@ljharb
Copy link
Member

ljharb commented Mar 2, 2016

@erights: per tc39/ecma262#438 (comment), do you think the future standardization of a stack on Error instances requires them to be unforgably brand-checkable?

@erights
Copy link

erights commented Mar 2, 2016

@ljharb I think it will cause them to be brand-checkable, in that the behavior of the api -- throw on non-errors -- will provide a brand check. OTOH, it may treat non-errors and errors from other realms identically, in which case it is not a brand check in the traditional sense. So I don't yet know.

@ljharb
Copy link
Member

ljharb commented Mar 2, 2016

Gotcha, thanks.

@ljharb ljharb mentioned this issue Mar 28, 2016
@ljharb
Copy link
Member

ljharb commented Mar 29, 2016

Thanks for this use case!

After discussion with the committee, this proposal is now withdrawn in favor of a proposal that standardizes Error stack traces.

@ljharb ljharb closed this as completed Mar 29, 2016
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

No branches or pull requests

5 participants