Giter Club home page Giter Club logo

proposal-error-cause's Introduction

Error Cause

Status: Stage 4

Author: @legendecas

Champion: @legendecas, @hemanth

Chaining Errors

Errors will be constructed to represent runtime abnormalities. To help unexpected behavior diagnosis, errors need to be augmented with contextual information like error messages, error instance properties to explain what happened at the time.

If the error were thrown from deep internal methods, the thrown error may not be straightforward to be easily conducted without proper exception design pattern. Catching an error and throwing it with additional contextual data is a common approach of error handling pattern. Multiple methods are available to augment the caught error with additional contextual information:

async function doJob() {
  const rawResource = await fetch('//domain/resource-a')
    .catch(err => {
      // How to wrap the error properly?
      // 1. throw new Error('Download raw resource failed: ' + err.message);
      // 2. const wrapErr = new Error('Download raw resource failed');
      //    wrapErr.cause = err;
      //    throw wrapErr;
      // 3. class CustomError extends Error {
      //      constructor(msg, cause) {
      //        super(msg);
      //        this.cause = cause;
      //      }
      //    }
      //    throw new CustomError('Download raw resource failed', err);
    })
  const jobResult = doComputationalHeavyJob(rawResource);
  await fetch('//domain/upload', { method: 'POST', body: jobResult });
}

await doJob(); // => TypeError: Failed to fetch

If the errors were chained with causes, it can be greatly helpful to diagnosing unexpected exceptions. As the example above shows, quite a few laboring works has to be done for a simple error handling case to augmenting the caught error with contextual message. Besides, no consensus on which property will representing the cause makes it not feasible for developer tools to revealing contextual information of causes.

The proposed solution is adding an additional options parameter to the Error() constructor with a cause property, the value of which will be assigned to the error instances as a property. So errors can be chained without unnecessary and overelaborate formalities on wrapping the errors in conditions.

async function doJob() {
  const rawResource = await fetch('//domain/resource-a')
    .catch(err => {
      throw new Error('Download raw resource failed', { cause: err });
    });
  const jobResult = doComputationalHeavyJob(rawResource);
  await fetch('//domain/upload', { method: 'POST', body: jobResult })
    .catch(err => {
      throw new Error('Upload job result failed', { cause: err });
    });
}

try {
  await doJob();
} catch (e) {
  console.log(e);
  console.log('Caused by', e.cause);
}
// Error: Upload job result failed
// Caused by TypeError: Failed to fetch

Compatibilities

In Firefox, Error() constructor can receive two optional additional positional parameters: fileName, lineNumber. Those parameters will be assigned to newly constructed error instances with the name fileName and lineNumber respectively.

However, no standard on either ECMAScript or Web were defined on such behavior. Since the second parameter under this proposal must be an object with a cause property, it will be distinguishable from a string.

Implementations

Polyfills:

JavaScript Environments:

  • Chrome, released on 93,
  • Firefox, released on 91,
  • Safari, released on 15,
  • Node.js, released on v16.9.0.

FAQs

Differences with AggregateError

The key difference between those two is that the errors in the AggregateError doesn't have to be related. AggregateError are just a bunch of errors just happened to be caught and aggregated in one place, they can be totally unrelated. E.g. jobA and jobB can do nothing to each other in Promise.allSettled([ jobA, jobB ]). However, if an error were thrown from several levels deep of jobA, the cause property can accumulate context information of those levels to help understanding what happened exactly.

With AggregateError, we get breadth. With the cause property, we get depth.

Why not custom subclasses of Error

While there are lots of ways to achieve the behavior of the proposal, if the cause property is explicitly defined by the language, debug tooling can reliably use this info rather than contracting with developers to construct an error properly.

proposal-error-cause's People

Contributors

devsnek avatar exe-boss avatar hemanth avatar keiya01 avatar legendecas avatar ljharb avatar mmkal avatar wangzongxu avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

proposal-error-cause's Issues

Potential footgun with 'cause' property / not accepting Error

Consider the following:

function MakeBreakfast() {
    try {
        MakeCoffee();
    } catch (err) {
        throw new Error("Error making breakfast", err);
    }
}

function MakeCoffee() {
    try {
        BoilWater();
    } catch (err) {
        throw new Error("Error making coffee", { cause: err });
    }
}

function BoilWater() {
    throw "Error boiling water";
}

Am I correct in saying that that in code would produce an Error like this

{ 
    message: 'Error making breakfast',
    cause: 'Error boiling water'
}

instead of the intended

{
    message: 'Error making breakfast',
    cause: {
     message: 'Error making coffee',
     cause: 'Error boiling water'
    }
}

?

Due to both the options parameter AND Error having a cause property.

For those familiar with similar features in other languages (eg. C#) this will be an extremely common mistake, and it won't always be immediately obvious as it's just one intermediate "cause" being missing. It'd also be difficult for TypeScript to pick up on a problem like this without explicit.

Stringifying the cause

The status quo of the error.toString is concatenating the error.name and error.message. Whilst most usage of the error string will require the stacktrace been formatted too, and the stack property has not been carved into the spec, it might be reasonable to not describe the error.toString with stack traces. However, I didn't find investment in proposal-error-stacks on the error.toString either.

As such, the options available will be:

  1. Defining the cause formatting in the error.toString. Nevertheless, the cause of an error is commonly appended after the detail of the error itself (it is less relevant than the error self's detail). That is to say, the cause needs to be appended after the stacktrace of the error. This requires the stack to be a dependency of this proposal.
  2. Defining the cause formatting in the stack string definition. Spec of proposal error stacks has described the content of error.stack.
  3. The formatting is left to implementations. Implementors may modify the formatting of an error for colorization or readability in different situations. Node.js has its own error formatting in util.inspect. Dev tools not even rely on the error.stack property now.

As changing the return value of error.toString may cause unexpected breakage to existing codes, I'm leaning to option 2, or 3 if it sounds not reasonable to add cause to the stack string.

Comparison with existing solutions

As the FAQ acknowledges, this can be implemented using custom error classes. The main value is therefore helping to coordinate debug tooling. Has a survey been done of existing tooling that could benefit from this proposal? If so, does this cover the important use cases?

Do we have examples in the wild of problems that would be solved by this proposal?

Options bag parameter

How about, instead of the single second parameter, an "option bag" object is used instead for extensibility later?

ex:

function foo(callback) {
    try {
        callback( );
    } catch( cause ) {
        throw new Error( "callback threw an error", { cause } );
    }
}

Rethrow?

The term “re-throw” is used in the explainer a few times, but the examples (and the proposal itself) seem to concern only throwing new errors, not rethrowing errors.

I can see how this proposal helps with a problem similar to rethrowing: both approaches allow you to retain the stack trace. It’s not clear to me what advantage there is to the indirection (you’ll need to “unwind” the chain of errors now to get the full trace), but in any case, I do not think “re-throw” as used here is what’s generally meant by that term.

// Not rethrowing:

try {
  foo();
} catch (err) {
  throw new Error('oops, I just obliterated the stack trace');
}

// Rethrowing:

try {
  foo();
} catch (err) {
  err.message += ' (some extra context)';
  throw err;
}

Aggregate Errors have a cause

Just want to make sure I flag this publicly. It doesn't make any sense to me for an AggregateError to have a cause. The "cause" of an AggregateError is no more and no less than the aggregated errors (and of course their causes).

Perhaps it would be appropriate for AggregateError to simply fill the cause property in with the errors array?

How will this conflict with existing solutions like VError?

I opened an issue in VError (TritonDataCenter/node-verror#81) about this spec as, while I very much look forward to this spec, it seems to be a possible issue that the VError extension of Error currently implements a different .cause than what this proposal proposes, and same goes for @netflix/nerror.

In VError:

err.cause().message

In this proposal:

err.cause.message
  1. Any current code that assumes that when .cause exists it will be of the style of VError#cause, will have an issue, as it will try calling something that isn't a function. This probably shouldn't be a problem as one always needs to ensure that one have complete checks, like the one in Bunyan.

  2. Any code that assumes that .cause will be as defined in this proposal won't have an issue with a VError#cause value, as it will then just be treated as if the method itself is the cause rather than the returned value of it – which is okay. (This can change if #21 arrives at something else)

  3. Type wise there will be an issue as any class that extends Error needs to have compatible such extensions, and the @types/verror definition of .cause is not compatible with the one in this proposal, so it will fail, as can be exemplified using .stack

I just want to raise this, partly as a way to connect the discussion in eg. VError to a discussion in this proposal.

Reserving error options for future use?

It may make sense to reserve error options for future use so that they don’t conflict with libraries – for example:

  • All error options that start with lowercase letters must not be used by libraries.

Path to stage 4!

Stage 4

  • committee approval
  • implement in two browsers
    • node
    • Chrome Canary / v8
      • Chrome (flagged)
      • Chrome (unflagged)
    • Firefox / SpiderMonkey
      • Firefox Beta
      • Firefox (unflagged)
    • Safari / Webkit / JSC
      • Safari (unflagged)
      • Safari Technical Preview
    • Edge / Chakra
      • Edge (unflagged)
    • TypeScript
    • Hermes
  • ecma262 PR approved
  • prepare ecma262
  • merge test262 tests
  • write test262 tests

Stage 3

  • committee approval
  • spec editor signoff
  • spec reviewer signoff from @ljharb and @codehag
  • receive implementor feedback

Stage 2

  • receive developer feedback
  • committee approval
  • spec text written
  • spec reviewers selected

Serialization

I realize this is kind of not what the spec is about, but also I think failing to consider this aspect of the proposal could lead to predictable situations which create unnecessary problems downstream.

The main thing I wonder is if I am writing an application in which I expect that my errors may have a cause property, do I need to print the cause myself?. Right now I don't know the answer. If I don't print the cause, this feature is diminished in usefulness as any cause data is lost as soon as the error is serialized for logging. If I do print the cause, I risk double-printing it if ever a standard mechanism is introduced for printing errors with all their relevant information such as cause.

What about a better Error spec ?

Currently, the Error class is not well defined due to its past history.

The only know things are:

  • first argument of the constructor is the message
  • properties: name and message

...and that's all. Every others properties (like stack) are not part of any specs.

What about a real spec ?

I would consider on my side, more something like this:

interface Error {
  name: string;
  message: string;
}

interface ErrorOptions {
  name: string;
}

interface ErrorConstructor {
  new(message: string, options?: ErrorOptions): Error;
}
interface ChainedError extends Error {
  cause: Error;
}

interface ChainedErrorOptions extends ErrorOptions {
  cause: Error;
}

interface ChainedErrorConstructor {
  new(message: string, options?: ChainedErrorOptions): ChainedError;
}

Consider having cause always present

The spec right now tries to use the existence of cause in the options bag to decide if cause should be present on the Error object.

This seems a bit weird, as we try in most parts of the language (and also the web platform) not to distinguish between "not present" and "present but undefined".

It also leads to annoying questions like, if I'm cloning an error, do I need to check for cause's presence? It'd be nicer if I could just do clone.cause = original.cause or, if I'm feeling fancy, Object.defineProperty(clone, "cause", Object.getOwnPropertyDescriptor(original, "cause")).

Finally, these sort of sometimes-one-shape-sometimes-another objects are pretty unusual for the ECMAScript spec. Although Errors are pretty unusual for the ECMAScript spec already, I think this extra bit of weirdness isn't necessary.

Playground

For stage-2 we would need to receive developer feedback.

@legendecas It would be great to have a playground where developers can throw new Errors and play around with the cause and give us feedback, maybe right in the playground.

Maybe we can use gh-pages for the playground.

For the polyfillish behavior, can we do something like the below?

class ErrorCause extends Error {
  constructor(cause = '', ...params) {
    super(...params)

    if (Error.captureStackTrace) {
      Error.captureStackTrace(this, CustomError)
    }
    this.cause = cause
  }
}

try {
  throw new ErrorCause('Failure', 'Caused due to a critical error')
} catch(e) {
  console.error(e.cause) 
  console.error(e.message)
}

HasProperty + Get combo doesn't seem right for option bags

https://tc39.es/ecma402/#sec-getoption is what Ecma 402 does: only a single Get(), with a default if the result is undefined. https://heycam.github.io/webidl/#es-dictionary similarly for the web platform.

The dual step means you'll trigger double the meta-object protocol traps. This is mostly a theoretical concern since if you have a weird proxy where [[GetOwnProperty]] and [[Get]] disagree, you're screwed anyway. But it feels inelegant.

However that doesn't seem to account for your desired semantics of distinguishing present vs. undefined-but-not-present. I argue in #35 this is bad semantics, so I think that'd be the best way to fix this :). I'm not sure if there are any others.

Printing an error with a cause

I think it would be important to tell the users in err.toString() that there is a cause attached to it.

Note that this has a precedence in https://www.npmjs.com/package/verror:

var err1 = new VError('something bad happened');
/* ... */
var err2 = new VError(err1, 'something really bad happened here');
 
console.log(VError.fullStack(err2));

Printing:

VError: something really bad happened here: something bad happened
    at Object.<anonymous> (/home/dap/node-verror/examples/fullStack.js:5:12)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:139:18)
    at node.js:968:3
caused by: VError: something bad happened
    at Object.<anonymous> (/home/dap/node-verror/examples/fullStack.js:3:12)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:139:18)
    at node.js:968:3

I think some similar machinery should be provided - if it does not become the default behavior of .stack.

What type of value is "cause" meant to contain?

The explainer talks about chaining errors, or catching and re-throwing with more contextual information. That implies cause should usually be an Error instance of some sort.

However, one of the proposal champions suggests using a string:

throw new Error('Request Error', 'Expected JSON got a string!');

Is the intention of cause to contain arbitrary "detail" data of this sort? The example goes a bit further, even, making the message property into more of a "title", where the cause property contains the actual error message.

I ask because web specifications have sometimes wondered about how to include more detailed error codes or similar (e.g. for distinguishing different types of network errors), and so if cause is meant to contain general, potentially-string data about what happened, we might start using it as such in web specifications.

draft spec feedback

message is conditionally defined, but Error.prototype.message exists, so 'message' in error and error.message always do something.

cause is conditionally defined, but Error.prototype.cause does not exist. I'd suggest defining it so that it matches message.

Reviewers

Hi, and congratulations on stage 2!

Because we were rushed, it looks like we missed reviewers for this. I can probably volunteer. Do you have other reviewers?

How should this interop with custom stack traces?

I'm not explicitly proposing https://github.com/tc39/proposal-error-stacks or any similar proposal be merged into this, but I do feel it should be taken into account as there's very much a valid reason to want to provide a custom stack trace (React already forges .stack for component stack traces - there's clear precedent for a need). This would only need one field, of course, but it's still worth accounting for.

There's also the noted interop issue with Firefox, and that could have relevance for this as well.

An options bag as proposed in #11 (unnecessarily closed IMHO) could work, but I don't want to imply that's the only answer to this question.

AggregateError: `InstallErrorCause` should be performed after IterableToList

I'm almost certain we discussed this before, but I'm implementing a polyfill and discovered this.

Errors from errors should be thrown before errors from options, since errors from the first argument should be thrown before errors from the third. Can we move the InstallErrorCause call down to be the last part of the AggregateError constructor, just like all the other NativeErrors?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.