812764 - Errors don't carry any meaningful information on them

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Victor Porof [:vporof][:vp]]

The only thing available on instances of Error is __proto__, which, as expected, contains empty strings for the name and message properties. This makes the pause-on-exceptions unusable in some cases and watch expressions to display strings instead of error objects for now.

See https://bugzilla.mozilla.org/show_bug.cgi?id=727429#c13

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Bumping the priority on this, since it's starting to get on my nerves.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: error-properties.patch

Sorry for stealing the bug, Eddy, but I was talking with a dev and he was really annoyed by this, so I looked into it, and I was able to fix it in the server layer really quick.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: error-properties.patch

Sorry, didn't add the test file in the last patch.

Comment 5

[Brandon Benvie [:benvie]]

It might be good to note whether _forceMagicProperties has been run on this object and bail out after the first time doing it.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 816784 [details] [diff] [review]
error-properties.patch

Review of attachment 816784 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you Nick, this was really important!

I still think fixing the platform to avoid the need for this workaround would be preferable, so if Eddy agrees, I would suggest that we file a followup for the better fix. I don't think it's useful for the Debugger.Object reflection of an error object to not have these properties eagerly fetched. XPCOM's Components.Exception is already reflected properly as a Debugger.Object, so a JS Error should too.

(In reply to Brandon Benvie [:bbenvie] from comment #5)
> It might be good to note whether _forceMagicProperties has been run on this
> object and bail out after the first time doing it.

I agree with Brandon here. The message and stack properties can be quite large in some cases, and the extra memory overhead from copying them across the engine boundary could be a problem with mobile devices.

::: toolkit/devtools/server/actors/script.js
@@ +2697,5 @@
> +    const MAGIC_ERROR_PROPERTIES = [
> +      "message", "stack", "fileName", "lineNumber", "columnNumber"
> +    ];
> +
> +    if (this.obj.class.endsWith("Error")) {

This wouldn't catch custom errors with funny names, would it? I think a better approach is to check that this.obj.proto && this.obj.proto.class == "Error" or at least this.obj.proto.class.endsWith("Error").

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://hg.mozilla.org/integration/fx-team/rev/f615c239b9be

Comment 8

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Panos Astithas [:past] from comment #6)
> Comment on attachment 816784 [details] [diff] [review]
> error-properties.patch
> 
> Review of attachment 816784 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank you Nick, this was really important!
> 
> I still think fixing the platform to avoid the need for this workaround
> would be preferable, so if Eddy agrees, I would suggest that we file a
> followup for the better fix. I don't think it's useful for the
> Debugger.Object reflection of an error object to not have these properties
> eagerly fetched. XPCOM's Components.Exception is already reflected properly
> as a Debugger.Object, so a JS Error should too.

Filed bug 927019

> 
> (In reply to Brandon Benvie [:bbenvie] from comment #5)
> > It might be good to note whether _forceMagicProperties has been run on this
> > object and bail out after the first time doing it.
> 
> I agree with Brandon here. The message and stack properties can be quite
> large in some cases, and the extra memory overhead from copying them across
> the engine boundary could be a problem with mobile devices.

Done.

> 
> ::: toolkit/devtools/server/actors/script.js
> @@ +2697,5 @@
> > +    const MAGIC_ERROR_PROPERTIES = [
> > +      "message", "stack", "fileName", "lineNumber", "columnNumber"
> > +    ];
> > +
> > +    if (this.obj.class.endsWith("Error")) {
> 
> This wouldn't catch custom errors with funny names, would it? I think a
> better approach is to check that this.obj.proto && this.obj.proto.class ==
> "Error" or at least this.obj.proto.class.endsWith("Error").

Subclasses don't inherit the magic properties.

Comment 9

[Victor Porof [:vporof][:vp]]

\o/

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f615c239b9be