-
Notifications
You must be signed in to change notification settings - Fork 0
Error conditions, retry, and Error.cause #11
Comments
An error subclass seems like the natural/standard way to communicate a finer grained error, then that can have a I always thought of |
But what if it's not an |
Note that this (examples of a non-error object) includes any Promise rejections, like |
Yeah, my understanding was cause was mainly aimed at stack stitching of multiple Error instances. It's not a generic way to smuggle in more data into a single field; creating actual fields on a subclass or expando seems better for that. |
That's not the motivation as I understand it; that's what AggregateError is more aimed at. |
Ok, so the option 5 here is something like: class RetriableError extends Error {
constructor(message) {
super(message);
Object.defineProperty(this, 'name', { value: 'RetriableError' });
}
} |
@domenic ... if we go the route of a new Error subclass, I'd like to make sure we can do it in a way that ensures we won't clash down the road with any web platform stuff. To that end, do you think there'd be any interest among browser implementors for either a new error type or standardized property on |
I'm not sure. My initial impression is that it seems rather difficult because it's ambitious. If I were to go through the hundreds of thrown exceptions on the web platform, I'd be surprised if they neatly fell into "retriable" and "non-retriable" buckets. E.g. for fetch() we purposefully avoid giving much programmatic access to information about network errors, for security reasons. (But maybe a single retriability bit would not be a security leak?) The places that seem most promising are I/O specs like fetch(), IndexedDB, and fs.spec.whatwg.org. Maybe file issues on those repos and see if this is a problem web developers have, and if they'd benefit from such a bit? And then security discussions could be had there too. |
I'll get an issue opened in the webidl repo to start the conversation. |
To my understanding, I'd find this feature would be better served with option 1 or option 2, as the additional information is describing the error instance itself, rather than their "correlated" error instances. For option 2, "Non-standard own properties" may be standardized. If this "retriable" bit doesn't impose security concerns, I'd believe it could be a benefit to other web APIs too.
IMO That being said, I think option 1 (or subclasses) and option 2 are valuable to be explored. |
I think using In case of an added property, I recommend using a global symbol name as a property, as everybody has been stitching properties on errors for years. I think you'd want to avoid naming collisions. Something like This would be a generalization beyond fetch. |
Then I would not expect
Sure and while I don't personally like non I would also be interested in hearing more about your use case or where the platform can decide for the application whether something is retriable or not. I guess the closest we have is |
Ok, from the discussion here, it's looking like the preferred choices are either (a) create a new |
while that’s certainly an option, the design of the feature, as well as the idioms of the language itself, was that a thrown value may be set as a cause unchanged, just like try/catch and Promises don’t wrap a non-error value in an error. |
I’d prefer a new error type; its semantically clearer to me versus slapping random extras onto another error. |
IMO a new error type doesn't work here, because the retriable property is potentially orthogonal to the error type, and JS is single-inheritance. The problem here is a bit more obvious if we consider what happens when we have two different properties we want to express. For example, let's say we both want to express "makes sense to retry" and "client error vs. server error" as properties. The only way to cover all cases via types is to have four different types: RetriableClientError, RetriableServerError, PermanentClientError, PermanentServerError. This is obviously not a great direction. If we had multiple inheritance, then we could have error types that inherit e.g. from both RetriableError and and ServerError to indicate both properties through types, but we don't have that. So in my opinion it has to be a property, not a type. |
It could still be one type that has multiple properties, or a single property that's an array of enum values, or something. |
@ljharb Hmm, so you mean like... we introduce some sort of |
Yep, exactly that. |
I guess the big question then is will we run into any cases where we want to throw an error marked retriable, but existing specs say that we need to throw a specific type of error (like |
I suspect if this type inherits from DOMException, and doesn't add anything that conflicts, that it'd be fine since WebIDL is more about interfaces than nominal types. |
That does seem like a problem. What if |
I'm not sure I understand - if the DOM specification changes to throw a different error then implementors would presumably subclass that. |
Right, but you can end up with clashes. Also risks reducing the ability for code to be reusable. |
@annevk true. The only real ergonomic solution to avoid clashes is for the web to commit to carving out ways it won’t ever conflict, so other runtimes have space to innovate freely. |
As I pointed out in the original post, every option here runs the risk of clashes. What I plan to do on Monday is open an issue in whatwg/fetch to propose either a new property on DOMException to indicate retriability, or introduce a new error type for that purpose and see if we can get consensus among browser developers. Failing that, I'd like to bring the question to tc39. In the meantime, for our immediate use case we'll likely just have to do something fairly ad hoc accepting the risk of possible conflicts later. The rather silly thing about not using For instance... try {
throw { retriable: true };
} catch (cause) {
// Falls perfectly within the expected use of cause described in this thread
throw new Error('boom', { cause });
} Which is only superficially different from:
As for a couple of the specific points, @kentonv says:
Again, that's what try {
throw new MyRetriableError();
} catch (err) {
// Wrap in a DOMException to meet the API expectations
throw new DOMException('Network failed', { cause: err, name: 'NETWORK_ERR' });
}
Can be, doesn't have to be. A property also has potential issues if not implemented consistently (insert reference to the current debate around adding |
This seems very doable. If you use a prefix like |
I did say “ergonomic” :-) |
Within Workers we have been having a discussion about how to communicate to users via Errors that the conditions leading to an error are temporary and that the user should retry their operation. The how and when to retry is not important here.
For example, a
fetch()
promise can fail for many reasons. The network path could temporarily be down, the URL could be blocked, the header could be malformated, etc. We want to be able to clearly indicate that the user can/should retry their operation without requiring that the user resort to parsing the error message.We have several possible paths forward, all of which have the same fundamental problem. We'd like to get consensus on which approach folks would find the most agreeable.
Option 1: New error types
Option 2: Non-standard own properties on
Error
Option 3: Using
cause
Option 4: Using
AggregateError
Option 5: ??
Other ideas?
Current Thinking
My current thinking here is to prefer Option 3, using the
cause
property.Specifically, pulling out to a logical level: The purpose of the
cause
is to communicate the reason for this error. That reason might be that anotherError
was thrown, or it might be that some other condition occurred. For instance, the network was down, or there was an internal error, etc. So let's differentiate betweenError
andCondition
.If I have a transient condition and want to communicate that the user should retry their operation, then I could logically do something like:
The challenge with this, of course, is interoperability. If workers chooses to use
cause
in this way but otherfetch()
implementations choose to usecause
in other ways then we can run into interop issues. To be clear, ALL of the options suffer from this exact problem.Proposal
The proposal I would like to make is to define a new
ErrorCondition
interface specifically for use withcause
Essentially (treat this as a discussion example to express intent... the actual proposal can be refined):
Note that this interface intentionally mimics
DOMException
with the inclusion of aname
andmessage
accessors.Example use (assuming the proposal to add
cause
toDOMException
goes through):To be clear, I don't really have strong opinions on exactly how we solve this use case. My only requirement is that we have a mechanism for reliably communicating transient/retriable conditions that is interoperable across runtimes.
Some questions
The text was updated successfully, but these errors were encountered: