Error "detail"

I'm making another pitch to improve Error now that proposal-error-cause has got to stage 4 and been archived. Another feature it would be great to add to Error natively - the ability to attach extra information to an error. Hypothetical usage example after this proposal:

throw new Error('Resource not found', {
  detail: {
    table: 'users',
    id: 'abc123',
    hint: 'Try calling createUser first',
    advice: 'Go outside more',
  }
})

The values in detail would just hang off the Error object, similar to if you did Object.assign(new Error('Resource not found', { detail: ... }).

This is similar to info in verror (which is now unmaintained).

It's better than using Object.assign, because it's standardised, so 1) the intent is clearer, 2) it's less weird - code-reviewers and linting tools won't object, 3) other tools like Sentry can usefully render the details, and can more reliably group errors based on the message (and, if desired, properties like detail.table).

It would be nice to have an official home for extra properties, as opposed to just sticking them on the error instance wherever we please. It means we don't have to worry about JavaScript adding a new built in property with the same name as what we were using, which, if happens might (might...) not break your code, but it is still confusing to all-or-a-sudden be accidentally shadowing built in properties.

1 Like

@theScottyJam I'm not sure if you're saying you like this idea or not? Do you mean going ahead with this would risk unexpected behaviour for existing code that does Object.assign(new Error(''), {detail: 123})? To me it seems pretty in line with proposal-error-cause.

Re the official home for extra properties, this was actually why I opened the PR which moved proposal-error-cause to an "options bag": Switch to options bag by mmkal ยท Pull Request #25 ยท tc39/proposal-error-cause ยท GitHub - I was thinking the second argument passed to the Error constructor would be that home.

I'm saying that I like this idea.

As things are today, I have to modify an error instance to add extra data to it, and then hope that tc39 doesn't later standardize and new property with the exact same name that I'm already using. If there was a new standard place for my arbitrary data to go (like you're suggesting), I wouldn't have to worry about that problem. It also just feels cleaner.

1 Like

Why isn't that place already "cause"? You can put an object there with whatever you want in it.

@ljharb because cause has a different semantic meaning than detail. My assumption was that usually cause will be an Error in the same way that usually something that's thrown is an Error. Not a requirement, but your Sentry instance is going to be more helpful if you stick with Errors.

Taking the example from my original post, but (ab)using cause:

It seems silly, would we really want to encourage writing code like that?

1 Like

I'm not sure why it's any better or worse to spell that property "cause" or "detail".

because cause has a different semantic

I think this is the key in @mmkal's point.

The error cause proposal proposal explains its usefulness by first stating that the pattern of "Catching an error and throwing [errors] with additional contextual data is a common approach of error handling pattern.", followed by explaining how there's no standard way to follow that pattern - thus, error causes are the standard provided for us, and the reason they exist is because they are "greatly helpful [for] diagnosing unexpected exceptions"

If the example error that @mmkal is describing got through to my program, I would (wrongly) assume that what happened was that an object literal got thrown, caught, and then put into a new error as the cause, then finally the new error got thrown. Because, semantically, that's what an error cause is supposed to mean. I certainly wouldn't expect a library author to be purposefully putting data related to the error in this "cause" field for my program to grab and use - especially since that data isn't the "cause" of the error, it's just additional information about the error.

1 Like

For these I think a subclass would satisfy these requirements, and a pattern I've seen used many times.

class ResourceError extends Error {
  constructor(message, { table, id, hint, advise...rest}) {
    super(message, ...rest);
    this.resource = { table, id };
    this.display = { hint, advise };
  }
}

This sounds like the more valuable part. Does sentry currently only inspect the message field?

1 Like

Subclassing isn't always the best option (though it is how I solve the problem today). Say I had subclasses the error class and added a new property called "cause". Then, whoops, tc39 decided to make that a standard property, and now anyone seeing an error with a "cause" property will assume certain semantics about it that may not have been true with my particular error.

A solution to that is always using either mutable own properties or getter/setter pairs for new proposals.

Even the error stack proposal includes a stack setter.

It's more about the conceptual conflict.

Similar to how Angular used to have a function named async, then later JavaScript added the async keyword, and while Angular's async function technically still worked, they still chose to deprecate it in favor of a new function that was exactly the same, but had a different name.

In a similar way, sure it might be possible to add conflicting properties to the base error class without technically breaking my code, but it makes my code really confusing.

@aclaymore a late reply:

Re subclassing: yes, that does allow attaching additional properties to errors, and is a a sensible pattern in many cases. But adding a new class to a codebase's cognitive scope just for the sake of adding this detail option isn't always what a developer wants to do. Sometimes it's preferable to add detail ad-hoc. Especially in the most common case - when an error is seen in production but has too little information to effectively debug. Let's say we are already throwing new Error('Upload failed', {cause: someOtherError}). But unfortunately we also need to know, say, the inputs to the function that saw the error. With a detail prop, we can just update the code to new Error('Upload failed', {cause: someOtherError, detail: params}) and we'll get more info next time the error occurs.

With the current subclassing solution, we need to refactor. We'd need to create a new Error subclass, probably in the module scope, maybe somewhere else, and we'd need to decide on the constructor parameters. The developer would, depending on their mood, use a constructor signature like constructor(message: string, {cause: unknown, detail: unknown}), or constructor(params: {message: string; cause: unknown; detail: unknown}) or constructor(message: string, options: {cause: unknown; extra: unknown}) or constructor(message: string, cause: unknown, detail: unknown). Likely most existing codebases have multiple variants in different places. The cognitive cost of these different signatures doesn't really serve any purpose, in the majority of cases (where it does serve a purpose, because extra is meaningfully different from detail, say, the ability to subclass wouldn't go away). So while subclassing helps mitigate the original problem, I still think this proposal is worthwhile - it stops forcing people into awkward/unnecessary/inconsistent code.

As for Sentry - the Sentry server version my team are currently on does seem to surface all enumerable properties hanging off an Error sent to it. But we are using a custom lightweight Sentry client, which currently only looks for the cause property. This isn't necessarily a design decision I agree with, but it's a somewhat understandable one for a client whose focus is efficiency and lightweightness. cause is the only one in the spec, that's worth looking for, right now. I'm proposing we add detail so that implementers are given a clear signal that it's worth including.

(@ljharb the above example new Error('Upload failed', {cause: httpError, detail: inputParameters}) is one that comes up in real life, and a simple example of why cause isn't a solution to this problem)

Not sure I fully understand your specific use case. We already have the ability to embed arbitrary information into the error message. I would imagine that this "detail" parameter is meant for programmatic usage - i.e. catching the error and inspecting information about it.

e.g.

try {
  ...send network request...
} catch (error) {
  if (error instanceof NetworkError && error.detail.statusCode >= 400) {
    ...
  } else {
    throw error;
  }
}

Before using the detail property programmatically, you would first need to know what kind of error you're dealing with, which means you would need to be throwing an error subclass - either that or perhaps attach some special distinguishing error code onto the error object - maybe that's what you're hoping to do? error.detail.code === "networkError" && error.detail.statusCode >= 400?

I also would imagine that the subclass error constructors would look more like constructor(message: string, options: { httpCode: number, responseHeaders: Record<string, string>, ...etc }), and then the error constructor would be in charge of constructing the details property based on what was passed in in the constructor - it may do some normailzation of the data, provide defaults, etc, as it builds the details object.

(I also don't know much about sentry, so that may be part of why I'm not fully getting it)

Yes, I am primarily thinking of error-logging tools rather than javascript code in the same realm catching errors. Basically I'm thinking of when there's no catch statement, so a tool that broadly catches all errors is going to report the error, and the developer wants to make sure whoever ends up debugging it has some extra information.

For catch statements like your example, you'll still be better off subclassing. This proposal offers an on-ramp to a full error subclass, once that use-case arises. With the added benefit of nudging towards a standardised detail property rather than arbitrary variants like info/extras/details etc. Of course it doesn't preclude more specific properties like code from being used in sub-classes where appropriate.

And, sorry, just some more clarifying questions - what's the advantage of using a detail property over embedding the information in the error message directly?

e.g. instead of

throw new Error('Received a bad HTTP code', {
  detail: {
    code: response.statusCode,
    requestUrl: request.url,
    requestMethod: request.method,
  }
});

you do:

throw new Error([
  'Received the bad HTTP code ${response.statusCode}.',
  `Additional details:`,
  `  requestUrl: ${request.url}`,
  `  requestMethod: ${request.method}`,
].join('\n'));

or

throw new Error('Received a bad HTTP code.\nAdditional details: ' + JSON.stringify({
  code: response.statusCode,
  requestUrl: request.url,
  requestMethod: request.method,
}, null, 2));

The message is often used as track errors. These errors:

throw new Error('Received a bad HTTP code', {
  detail: {
    code: response.statusCode,
    requestUrl: request.url,
    requestMethod: request.method,
  }
});
// ...
throw new Error('Received a bad HTTP code', {
  detail: {
    code: response.statusCode,
    requestUrl: request.url,
    requestMethod: request.method,
  }
});

could count as 2 instances of the same 'Received a bad HTTP code' error whereas these errors

throw new Error([
  'Received the bad HTTP code ${response.statusCode}.',
  `Additional details:`,
  `  requestUrl: ${request.url}`,
  `  requestMethod: ${request.method}`,
].join('\n'));
// ...
throw new Error([
  'Received the bad HTTP code ${response.statusCode}.',
  `Additional details:`,
  `  requestUrl: ${request.url}`,
  `  requestMethod: ${request.method}`,
].join('\n'));

could be seen as 2 instances of two different errors 'Received a bad HTTP code <this-data>' and 'Received a bad HTTP code <that-data>'. That can throw off the numbers when you're trying to look at the number of occurrences for specific errors.

Isn't that one of the uses of subclasses/error codes? To make it easier for programs to distinguish between different types of errors?

In my mind, the error message is a place whose primary audiance is programmers, and so we should feel free to put whatever we need in that error message, and change it as often as we'd like, etc, in order to improve the debugging experience. You can try to programmatically analyze error messages if you want, but by nature, it should be understood that programmatic analysis will be fairly unstable and a difficult thing to do, because error messages are not supposed to be written for computers to understand.

If we additionally want to make our errors easy for computers to analyze, then we place that information into spots that's easy for a computer to grab at, like error codes, subclasses, and other properties on the error instance. The primary audiance for these additional fields would generally be computers. humans could use that information to help them debug, but it's not necessarily tailored-made for humans to use.

Suggesting that dynamic information be left out of error messages in order to make the error messages easier for computers to understand, and instead we place that helpful debugging information into properties feels completely backwards to me, due to my above understanding of how errors are generally built.

In my experience, such error log searching you're likely using regexps, not exact matches anyways. You're not gaining much IMHO if that's your goal.

You're misunderstanding @senocular's use case. A system health dashboard will likely, by default and without additional configuration, aggregate all Error objects with the exact same message so as to surface the ones that are occuring more frequently and thus are less likely to be PEBKAC. Including dynamic content in the message could cause them not to aggregate, and thus could lead to the errors not surfacing properly. Not to mention, requiring developers to JSON.stringify or template-literal their debugging information is a big and frustrating ask if you're trying to track down an elusive failure and potentially adding this sort of information to dozens of error sites, each of which might have useful ancillary data of different types - and JSON.stringify doesn't work on everything.

What OP is asking for here is, as I understand it, a standard place to put untyped, arbitrary data that would be useful to a human diagnosing the Error, and I wholeheartedly support the idea. message isn't appropriate, since it's typed as a string. cause isn't appropriate, as it has the semantic meaning of "the Error that caused this one to throw". Subclasses are fantastic when you're defining data that might be of programmatic use, but they carry a significant burden both to readability and maintainability when used for generic error-detail inclusion.

To elaborate on the maintainability bit: if I'm new to a codebase and I see that someone has gone to the trouble to subclass Error and throw the subclass, I have to assume that both the particular class and the particular data types of its properties are being used programmatically - and thus, I should not modify them. The message parameter/property, on the other hand, is generally well-understood not to have programmatic meaning other than, as noted, to aggregate identical ones. Thus, if I create a new Error subclass, even if it's only intended to provide diagnostic information to humans, I am adding to the maintenance burden of the codebase.

For my part, I have an extremely specific use-case for supporting the detail property, which may help to shine some light on the benefit: the console.error; throw new Error pattern. I find myself doing something along the lines of the following with some frequency:

if (!this._backend.checkOptions(this._options)) {
	console.error("checkOptions returned false on a newly-constructed backend",
                  options, this._backend, this._options);
	throw new Error("checkOptions returned false on a newly-constructed backend");
}

The throw new Error is for breaking the control flow, but that message is fairly useless to me when I see it in the devtools console on my browser. The console.error, on the other hand, lets me output useful information to the console in a form that lets me expand and manipulate the options and backend objects, but it doesn't break control flow. I would strongly prefer being able to simply write the following:

if (!this._backend.checkOptions(this._options)) {
	throw new Error("checkOptions returned false on a newly-constructed backend",
                    {details: {options,
                               backend: this._backend,
                               parsedOptions: this._options}});
}
1 Like