Describe the problem
The purpose of handleError is twofold:
- it gives you a way to identify bugs in your app (by e.g. logging stack traces)
- it allows you to turn an
unknown error into an App.Error
In the case of things like 404s and 413s, there is no point in logging the stack trace. It doesn't indicate a bug in the app, it indicates user error. There isn't really much for the app developer to do. As such, running handleError on it is a bit pointless.
Having said that, if App.Error is defined as more than just { status, message }, the rendered error will be missing those properties if it isn't passed to handleError.
Having said that, the same will be true if the app developer doesn't implement handleError. So arguably the types are a lie anyway.
Describe the proposed solution
Honestly I'm not 100% sure on the right course of action here, except for #16411 which I think is an obvious win.
I'm somewhat leaning towards the following: instead of App.Error being a superset of { status, message }, we make it entirely user-defined, and add an additional type (RenderedError?) which is basically this:
type RenderedError = { status: number; message: string } & Partial<App.Error>;
With that in place, we would no longer be lying about the types when handleError isn't defined, and we would be able to avoid passing unexpected-but-internal errors to handleError. The third argument to error(...) would be optional, with no need for the overload.
Having said that I can imagine the following responses:
- "I don't want to deal with a
RenderedError with its optional properties, I want an App.Error"
- "If the types are a lie because someone didn't implement
handleError, that's their problem"
We could solve this by adding another hook (transformError?) but that doesn't feel great either.
Alternatives considered
Please tell me
Importance
nice to have
Additional Information
No response
Describe the problem
The purpose of
handleErroris twofold:unknownerror into anApp.ErrorIn the case of things like 404s and 413s, there is no point in logging the stack trace. It doesn't indicate a bug in the app, it indicates user error. There isn't really much for the app developer to do. As such, running
handleErroron it is a bit pointless.Having said that, if
App.Erroris defined as more than just{ status, message }, the renderederrorwill be missing those properties if it isn't passed tohandleError.Having said that, the same will be true if the app developer doesn't implement
handleError. So arguably the types are a lie anyway.Describe the proposed solution
Honestly I'm not 100% sure on the right course of action here, except for #16411 which I think is an obvious win.
I'm somewhat leaning towards the following: instead of
App.Errorbeing a superset of{ status, message }, we make it entirely user-defined, and add an additional type (RenderedError?) which is basically this:With that in place, we would no longer be lying about the types when
handleErrorisn't defined, and we would be able to avoid passing unexpected-but-internal errors tohandleError. The third argument toerror(...)would be optional, with no need for the overload.Having said that I can imagine the following responses:
RenderedErrorwith its optional properties, I want anApp.Error"handleError, that's their problem"We could solve this by adding another hook (
transformError?) but that doesn't feel great either.Alternatives considered
Please tell me
Importance
nice to have
Additional Information
No response