Closed Bug 1679653 Opened 4 years ago Closed 4 years ago

Implement the Error Cause proposal

Categories

(Core :: JavaScript Engine, enhancement, P3)

Product:
Core
Component:
JavaScript Engine
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
91 Branch
Tracking Flags:
Tracking Status
firefox91 --- fixed

People

(Reporter: alex.fdm, Assigned: anba)

References

(Blocks 2 open bugs,
URL
)

Details

(Keywords: dev-doc-complete)

Attachments

(7 files)

Bug 1679653 - Part 1: Implement the error cause proposal. r=yulia!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1679653 - Part 2: Update test262 feature tags. r=yulia!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1679653 - Part 3: Update test262 to pick up error cause tests. r=yulia!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1679653 - Part 4: Update test262 exclusions. r=yulia!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1679653 - Part 0: Free-up a slot on ErrorObject. r=yulia!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1679653 - Part 5: Add xray support for the Error cause property. r=peterv!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1679653 - Part 6: Print error cause in shell error reporter. r=yulia!
48 bytes, text/x-phabricator-request
Details | Review

The proposal is currently in Stage 2.

Severity: -- → N/A
Priority: -- → P3

Proposal now in Stage 3.

Blocks: es-proposals-stage-3
No longer blocks: es-proposals-stage-2
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED

Depends on D115234

@evilpie noticed that the EXNTYPE_SLOT is no longer needed and instead can
be replaced by comparing an error's class against ErrorObject::classes.

Drive-by change:

  • Strengthened the assertion in ErrorObject::init to test against type < JSEXN_WARN.

Gentle review ping for https://phabricator.services.mozilla.com/D115353.

Flags: needinfo?(peterv)
Flags: needinfo?(peterv)
Pushed by andre.bargull@gmail.com: https://hg.mozilla.org/integration/autoland/rev/577a29a4b7ba Part 0: Free-up a slot on ErrorObject. r=yulia,evilpie https://hg.mozilla.org/integration/autoland/rev/be1e81db2f62 Part 1: Implement the error cause proposal. r=yulia https://hg.mozilla.org/integration/autoland/rev/728e142d5ecd Part 2: Update test262 feature tags. r=yulia https://hg.mozilla.org/integration/autoland/rev/8704e075c2db Part 3: Update test262 to pick up error cause tests. r=yulia https://hg.mozilla.org/integration/autoland/rev/d3caa5a42a29 Part 4: Update test262 exclusions. r=yulia https://hg.mozilla.org/integration/autoland/rev/55cb6696ad3b Part 5: Add xray support for the Error cause property. r=peterv https://hg.mozilla.org/integration/autoland/rev/3093ea3c4c4b Part 6: Print error cause in shell error reporter. r=yulia

FF91 Docs work for this is can be tracked in https://github.com/mdn/content/issues/6714

Some questions:

  1. Is this available release 91 build or is it restricted (e.g. to nightly)?
  2. I can see from the spec and this example how this is generally supposed to be used. Essentially you catch the original error(s) and throw a new error with a more useful message, passing the original error as the cause.

What I don't really understand from the examples is why that is useful. The examples still use the original message for differentiating the particular error from all errors that might be raised (or they could, with a trivial restructure), so why do you care what the original error object was? By contrast I can see how a custom error is useful, because it is simple to write handlers that check the error type and we can do this without having to contrive unique message names.

Further, the spec and example talk about cause giving us an agreed way to identify the cause of an error in order to improve the presentation of the cause by developer tools. But the cause doesn't have to be the original error object - it can be anything. So if I decide to put some string "This is the cause" in there how is this in any way useful for tools to present the error cause?

My "guess" from https://bugzilla.mozilla.org/show_bug.cgi?id=1711887 is that we expect this to always contain the original error. So what this does is allow us to create a daisy chain of errors. Effectively a bit like unwinding a stack trace.

I hope that makes some sense - I can see what it does and how you might use it, but the examples feel very contrived and don't seem to add anything much over just having useful error messages. What am I missing?

Flags: needinfo?(andrebargull)

(In reply to Hamish Willee from comment #12)

Some questions:

  1. Is this available release 91 build or is it restricted (e.g. to nightly)?

It will be available in the release build.

  1. I can see from the spec and this example how this is generally supposed to be used. Essentially you catch the original error(s) and throw a new error with a more useful message, passing the original error as the cause.

What I don't really understand from the examples is why that is useful. The examples still use the original message for differentiating the particular error from all errors that might be raised (or they could, with a trivial restructure), so why do you care what the original error object was? By contrast I can see how a custom error is useful, because it is simple to write handlers that check the error type and we can do this without having to contrive unique message names.

For example it can be used to throw your own custom Error type. Let's say there's a fallible call into some framework code and for some reason it's necessary to wrap any errors into a different Error object. Using the cause proposal, it's possible to wrap an error while still keeping the original error (and its stack trace).

try {
  framework();
} catch (err) {
  throw MyError("new message", {cause: err});
}

Further, the spec and example talk about cause giving us an agreed way to identify the cause of an error in order to improve the presentation of the cause by developer tools. But the cause doesn't have to be the original error object - it can be anything. So if I decide to put some string "This is the cause" in there how is this in any way useful for tools to present the error cause?

JavaScript allows to throw any value and therefore it's not possible to restrict the type of the cause property to be an object. If the cause property doesn't provide any useful information, Devtools can simply choose to ignore the property.

// The |throw| statement can throw any value.
throw "string";
throw 123;
throw undefined;

My "guess" from https://bugzilla.mozilla.org/show_bug.cgi?id=1711887 is that we expect this to always contain the original error. So what this does is allow us to create a daisy chain of errors. Effectively a bit like unwinding a stack trace.

I hope that makes some sense - I can see what it does and how you might use it, but the examples feel very contrived and don't seem to add anything much over just having useful error messages. What am I missing?

I think the feature will most likely end up being used when:

  • You don't control the original error's message.
  • Or you have to throw a specific error type.
Flags: needinfo?(andrebargull)

Thanks very much. I have finished the draft docs: https://github.com/mdn/content/pull/7316 - comments welcome.

The doc says that you can use the cause in a custom event too. Is that correct?

The doc says that you can use the cause in a custom event too. Is that correct?

Did you mean "custom error"? In that case, yes, it's possible to use cause there, too. The cause property is processed in the Error constructor, so any subclass just needs to pass the options in the super call:

class MyError extends Error {
  constructor(/* some arguments */) {
    // Needs to pass both |message| and |options| to install the "cause" property. 
    super(message, options);
  }
}

It won't work if the code is written like:

class MyError extends Error {
  constructor(/* some arguments */) {
    // Only passes |message| because that used to be the only argument for Error.
    // This code needs to be updated to pass |options|, too.
    super(message); 
  }
}

Thanks very much - just what I needed.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: