[HN Gopher] Code Design Decision - Always throw custom exceptions
___________________________________________________________________
Code Design Decision - Always throw custom exceptions
Author : that_guy_iain
Score : 70 points
Date : 2022-12-19 14:25 UTC (8 hours ago)
(HTM) web link (github.com)
(TXT) w3m dump (github.com)
| julik wrote:
| In short: if you have a meaningful recovery pathway for a
| particular exception this can be useful, but I found that 9 out
| of 10 exceptions/errors in code cannot be recovered from.
|
| This is checked exceptions all over again... Frankly, I usually
| do exactly the opposite. Most cases I've seen the exceptions that
| can arise are not widely known in advance, and for most spots
| where exceptions can be raised from I simply do not care about
| them as much either. If my DB is not accessible - do I really,
| really need to wrap it? If the DNS resolver is failing - do I
| really really need to wrap it?
|
| When debugging I actually appreciate when a
| library/controller/module/whatnot does not attempt to "enhance"
| the exception from something outside of its control with a
| wrapper, and value seeing the original failure instead.
| gadflyinyoureye wrote:
| > If my DB is not accessible - do I really, really need to wrap
| it?
|
| I would say yes. Most of your code doesn't care that the
| database failed with a PG-300292-AB error. What you do care
| about is that there is a system failure. If you wrap your
| system failure, and document it as THE exception, the API
| caller will know what to look for.
|
| In general there really are only a few exceptions: system,
| invalid input, non found. I'm probably missing one or two
| others. Most exceptions/errors are one of these. If you make
| these 3-5 exceptions/errors explicit, you're APIs will be
| pretty clear. A function can pretty much always returns a
| system exception (bad IO, sunspots, etc.). It can also throw an
| invalid input exception. Your client can handle these cases
| differently. For a HTTP RPC service, you return a 500 for
| system and 400 or 422 in the example. Not bad.
|
| One can even wrap the exception with a more detailed message
| (which I know you said you didn't like) that preserves flow. So
| you get a invalid input exception, you can add details around
| the null value by properly nesting the messages. They should
| show up in your logs.
| robertlagrant wrote:
| Not that this invalidates your point in any way, but
| returning a 503 for database (or any other resource)
| unavailable is useful to disambiguate server errors from
| transient connectivity/database is temporarily unavailable
| errors.
| teddyh wrote:
| > _I_ [...] _value seeing the original failure instead_
|
| This is what Python's "raise from" is for; to preserve the
| original exception.
| Sakos wrote:
| We primarily use custom exceptions to add detail to our error
| logs while still remaining concise. Generic exceptions are
| exceptionally useless for trying to debug issues, I've found.
| P5fRxh5kUvp2th wrote:
| Typed exceptions convey a bit of information to enable
| specialized handling at runtime, they are not meant to try
| and make debugging easier.
| Sakos wrote:
| I'm not sure what you mean by "they're not meant to". They
| work, I don't care what they're "meant to" do. It also
| helps with general analysis of logs.
| P5fRxh5kUvp2th wrote:
| It's redundant.
|
| The message (a string) will give you that information,
| nothing is lost by catching a base Exception and writing
| it to a log.
|
| typed exceptions is about special handling. The type
| information allows the programming language to offer
| special handling when that is necessary, it is NOT meant
| to replace good error messages.
|
| But it also isn't about taxonomy, which is something that
| a lot of people miss. It's putting the cart before the
| horse.
|
| ---
|
| But you specifically talked about debugging. If you're
| talking about interacting with an attached debugger, the
| developer already has everything they need, there is no
| reason why specific, typed, exceptions are needed to
| assist that.
|
| so you MUST be talking about general debugging by reading
| logs and the like, but those logs will tell you both the
| error message and the stack trace with the specific line
| that originally threw. Unless, of course, you're catching
| and rewrapping the exception and throwing that
| information away...
| rileymat2 wrote:
| > This is checked exceptions all over again...
|
| That is an interesting topic, as I went through the cycle of
| adding and removing them, they always felt like the right
| thing, but they felt like "work". They make coding less fun,
| having to think through failure conditions, like you say, you
| don't care about.
|
| I guess the answer is really there is no one sized fits all
| solution, we have some applications where the correct solution
| is to crash, we have other applications that can't crash under
| any condition. You just need to know where you are.
|
| Edit:
|
| Also, out of the box C++ checked exceptions implemented
| terribly.
|
| http://www.gotw.ca/publications/mill22.htm
|
| The (Mis)understanding section describes it pretty well.
| aleksiy123 wrote:
| I think this only makes sense if the 3rd party is also throwing
| custom exceptions.
|
| If you want to reduce coupling you should avoid throwing custom
| exceptions at all. Semantic information can go in the error
| message and log. The error type should be used to indicate to
| your program whether an error is recoverable, retriable or some
| other action needs to be taken. For example google on has 16
| canonical error codes for all APIs.
|
| https://github.com/googleapis/googleapis/blob/master/google/...
| CharlieDigital wrote:
| Addendum: write a useful exception message for future travelers
| that might need to review that stack.
|
| What was null? Why might it be null? Mis-configuration? Missing
| configuration? Can't load some data or connect to some system?
| Which setting should be verified? Give simple hints on what might
| have gone wrong to help future you.
| andix wrote:
| I agree with the first sentence. Your code should throw custom
| exceptions.
|
| But it shouldn't wrap other exceptions, if they are obvious. If
| library.readconfigfile(path) throws an IO exception while reading
| the file, just let it bubble up to the caller and don't bother
| handling it. Just make sure your internal state is clean
| (catch..finally)
| DotaFan wrote:
| I only throw exceptions to the flows that are not in my control
| (backend API, external libraries, IO).
| klntsky wrote:
| This is a reasonable approach, but having a catch-all case that
| handles the exception and re-throws it, but wrapped in custom
| exception type can be problematic in case a new exception is
| added - the consumer of the API needs to go over their code and
| change the types everywhere, or else their catch code will not be
| working as before anymore.
|
| This problem can easily go unnoticed, judging by my Haskell
| experience.
|
| The solution would be to always use "checked exceptions" (or
| similar concept in your language, e.g. `ExceptT SomeEnumType` in
| Haskell) and never use catch-all cases (or wildcard patterns, in
| case of Haskell), so that every exception handling case is tagged
| with exception type, and type-checked.
| MrTortoise wrote:
| Amazes me that so many people dont understand that if all things
| are dependant upon your business logic or on your domain then
| this is natural. Its natural hex architecture
|
| but this is far nicer way of saying the same thing
| https://ericlippert.com/2008/09/10/vexing-exceptions/
|
| I actually do not think your code should throw exceptions. It is
| really just an Either / result and then if something does blow
| its because you havent anticipated it via wrapping something in
| an either or result ... and so it should blow and the callers of
| your library should be submitting a defect..
| BurningFrog wrote:
| This feels like a classic case of looking at benefits but not
| costs.
|
| In that mindset, any non zero benefit to X means you must do X.
| Traubenfuchs wrote:
| Always avoid throwing custom exceptions. Example:
| ResponseStatusException in the spring framework.
|
| You can at any point throw ResponseStatusException + http status
| code + optional message, for example throw new
| ResponseStatusException(BAD_REQUEST, "resource must be in status
| EDITABLE, but is in status " + status);
|
| You can also define an error handler for this exception type to
| convert it in whatever error response format you desire.
|
| Writing custom exceptions for everything is the definition of
| overengineering, falls into the YAGNI trap and means kicking KISS
| with your feet.
| [deleted]
| xwowsersx wrote:
| One of the ways*
| hbrn wrote:
| Just because it's useful to wrap exceptions in 5% of the cases
| doesn't mean you should wrap the rest 95% _just in case_. YAGNI.
|
| 1. You don't know which exceptions will be raised in advance.
| Anything that involves IO can fail in a plethora of ways, and you
| don't even know which calls involve IO (e.g. a library might
| choose to cache something on disk).
|
| 2. Consumers of your code will not know how to deal with those
| exceptions.
|
| 3. Most of exceptions are unrecoverable (that's why they are
| called exceptions), the best course of action is to crash, which
| happens by default.
|
| 4. You debug those exceptions by looking at stack trace. Adding
| extra levels just to give a fancy meaningless name to an
| exception does not help.
|
| 5. The whole point of exceptions is to propagate. Parthenon
| essentially suggests converting exceptions into return values.
| zaphar wrote:
| I think there are two kinds of exceptions in languages that
| have them as their error handling mechanism.
|
| 1. Exceptions you expect your consumers to handle.
|
| 2. Exceptions you don't expect your consumers to handle.
|
| The first one I would argue you should wrap third party
| exceptions. There is in nearly every case important context in
| your code that the thrower of the third party exception will
| not know and whoever is reading or handling the exception will
| want to know.
|
| The second one should do whatever the equivalent of crashing is
| for your use case. Either exiting the program hard or bubbling
| to some top level handler where a "Something weird and
| unexpected happened and we can't continue whatever action was
| going on" alert or log get's recorded and the activity is
| terminated.
|
| If something is throwing an exception and you don't know it
| _can_ be throwing an exception it probably belongs in category
| 2. You may over time transition it to category 1 when you
| figure out that it is actually handleable.
|
| In my experience though if you aren't disciplined then every
| exception ends up being lumped into category 2 whether it
| should be or not. Any language that helps you force the
| categorization gets bonus points from me.
| bcrosby95 wrote:
| This is one of the problems with checked exceptions. The
| library author is in no position to expect me to handle an
| exception. Whether or not I can relies on the design of _my_
| system, which they have no window into.
| rowls66 wrote:
| That is true, so the library should throw the checked
| exception, and if the caller has no way to handle it, it
| should wrap the checked exception in an unchecked exception
| and throw the unchecked exception. Not too hard, and
| library clients that can handle some checked exceptions
| will be able to.
|
| I hate libraries that only throw unchecked exceptions. It
| seems easier initially, but makes writing correct code more
| difficult.
| Quekid5 wrote:
| > If the caller has no way to handle it, it should wrap
| the checked exception in an unchecked exception and throw
| the unchecked exception.
|
| Not you have a new problem: How is the code calling the
| _caller_ supposed to know about that exception? You can
| 't even catch it (even if you know about it!) using
| normal try-catch because what you have to do is catch the
| _wrapper_ exception and then check _inside that_ for the
| exception type you expect via instanceof.
|
| Checked exceptions (at least as implemented in Java) are
| _horrible_ for the composability of code wrt. error
| handling.
|
| That is why most libraries eschew checked exceptions
| these days. Unfortunately, large bits of the standard
| library in Java forces their hand wrt. re-wrapping stuff
| like InterruptedException and IOException and the like.
|
| (Not to mention, most of the time you really shouldn't be
| catching exceptions in very small scopes _or_ at the very
| highest level in your code. Involving every single layer
| in between is madness.)
| zaphar wrote:
| The error here is not that they are wrapping the
| exception necessarily. It is that they re-threw it as an
| unchecked exception. I strongly believe the only good use
| of unchecked exceptions is if the code should do the
| closest reasonable thing to crash safely. Everything
| should be clearly communicated to the callers so they can
| make good decisions about what to handle here and what to
| pass up the chain.
|
| If the complaint is that you then have too many different
| unchecked exceptions perhaps the error domain has been
| improperly modeled and you are getting a clue that the
| system is poorly designed.
| zaphar wrote:
| If it's an unchecked exception experience says I have a
| 60-70 percent chance of not knowing it's there without a
| careful reading of the code. Which means I can't know
| myself if it's possible to handle it or not. I can always
| rethrow an exception if I know it exists. Languages that
| give me a way to ensure I know about it means I avoid
| unnecessary pain later in production with someone breathing
| down my neck to please fix it now. I'll take that any day
| over a little boilerplate.
| stickfigure wrote:
| And 99.9% of the time the client is just going to catch
| generic Exception and doesn't care what the type is.
| Rollback a transaction, return 500 or display an error
| dialog. Done. It doesn't matter what the client library
| thinks.
| Groxx wrote:
| This is the problem of _Java 's implementation_ of checked
| exceptions... because they are not generic. Which forces
| middle layers to make impossible decisions like this.
|
| If the type system were more capable, they'd just be
| equivalent to typed generic error returns, which work fine.
| TedDoesntTalk wrote:
| I mean, can't you just catch Throwable (base class) and
| be done with it?
| CGamesPlay wrote:
| I'm reminded of an old Eric Lippert post about this. He
| basically says the same as you: boneheaded and fatal
| exceptions are not meant to be caught, and vexing and
| exogenous exceptions are ordinary flow control.
|
| https://ericlippert.com/2008/09/10/vexing-exceptions/
| p1necone wrote:
| "Exceptions you expect the consumer to handle" should be
| explicit return values on the function signature instead, via
| an Either/Result type or similar.
|
| Unless you're writing Java where you can force certain
| exception types to be handled, but people never write that
| code properly.
| zaphar wrote:
| Very few languages actually support Either or Result types
| ergonomically. I don't have a problem with people using
| exceptions for these but I do think if the language has
| compile time type checking it should provide a distinction
| between checked exceptions and non checked exceptions. I am
| very much in the minority here but I find it very useful to
| let robots tell me I forgot something than to discover I
| forgot it in production. I think this is the useful sort of
| lazy rather than the not useful sort. Not useful laziness
| is letting your customers discover the problem.
| p1necone wrote:
| Yeah I agree. I think the problem with checked exceptions
| in Java is a combination of misuse on both the consuming
| and the throwing end. They're amazing when they provide a
| compile time checked way to make sure consumers handle
| stuff _they actually definitely want to handle_ , but if
| you add too much noise then all the consuming code is
| just going to catch and rethrow even the important ones.
| eweise wrote:
| "Just because it's useful to wrap exceptions in 5% of the cases
| doesn't mean you should wrap the rest 95% just in case"
|
| Yes you should wrap ALL third party exceptions for the reasons
| given in the post. There is no 'just in case' reason. Any third
| party exception returned may cause your client to be dependent
| on it.
| charcircuit wrote:
| 4. When you catch the third party exception you typically throw
| away the stack trace which can make it harder to debug.
| hbrn wrote:
| All languages I know of have ways to preserve stack trace.
| That said, engineers tend to forget about it, so it is
| somewhat valid.
| drewcoo wrote:
| > The whole point of exceptions is to propagate.
|
| I thought the point of exceptions was what we're now calling
| observability.
|
| If somehow wrapping them in your own goo because you, the dev
| of that code, can help diagnose problems, more power to you.
|
| But most of the time that doesn't really help so letting them
| bubble up and not obscuring root causes is better.
|
| So . . . Use judgement. If you don't have judgement, ask a
| friend. If you don't have a friend, just let the exceptions
| propagate on their own.
| eweise wrote:
| I can't agree with any of these points.
|
| 1. You don't know which exceptions will be raised in advance.
| Anything that involves IO can fail in a plethora of ways, and
| you don't even know which calls involve IO (e.g. a library
| might choose to cache something on disk).
|
| Not sure how this relates to the code design decision
|
| 2. Consumers of your code will not know how to deal with those
| exceptions.
|
| Not sure the point here but if the argument is that the caller
| won't know how to deal with the third party exception, I
| disagree. Typically it just needs to return a single exception
| type that wraps any underlying cause. Caller can just either
| decide to deal with it or rethrow.
|
| 3. Most of exceptions are unrecoverable (that's why they are
| called exceptions), the best course of action is to crash,
| which happens by default.
|
| If the database is unresponsive do you want to crash your
| program? I wouldn't. I'll usually retry until its available.
|
| 4. You debug those exceptions by looking at stack trace. Adding
| extra levels just to give a fancy meaningless name to an
| exception does not help.
|
| Really don't think an extra trace in the stack is a reason to
| not create a well defined contract.
|
| 5. The whole point of exceptions is to propagate. Parthenon
| essentially suggests converting exceptions into return values.
|
| I didn't see anywhere in the doc where they mentioned
| converting exception into return values. Wrapped exceptions are
| still exceptions.
| mcguire wrote:
| 6. It is much faster and easier to develop an application's
| "happy path" while completely ignoring failures. Failure
| handling is a software maintenance issue. YAGNI.
| P5fRxh5kUvp2th wrote:
| > Just because it's useful to wrap exceptions in 5% of the
| cases doesn't mean you should wrap the rest 95% just in case.
| YAGNI.
|
| I'm going to curse here, but ... fucking seriously.
|
| If I get a .net ADO exception coming out of a 3rd party library
| it's absolutely not going to shock me or throw me for a loop.
| But do you know what IS a pain in the ass? Using 3 different
| libraries, all that wrap that same ADO exception in their own
| custom exception.
| z3t4 wrote:
| What do you use libraries for if not for the abstraction
| layer?
| P5fRxh5kUvp2th wrote:
| Well it certainly isn't so I don't have to understand what
| an ADO exception is.
| hbrn wrote:
| Haha, that's actually a good point. A single problem can
| affect dozens of modules, each having a different wrapper.
|
| If my db goes down, I very much prefer to see a single
| DatabaseConnectionFailed than a multitude of
| FailedToSaveObject, DatabaseError, CannotLoadData, IOError,
| SomethingIsWrongThisShouldNeverHappen, DBHostUnavailable all
| over the place. Good luck navigating through the noise and
| isolating those.
| eweise wrote:
| The article talks about solving that exact issue by
| wrapping all of those different exceptions so that you
| don't have to deal with them. Much easier to catch a single
| ThirdPartyInternalFailure exception than catch all the
| internal exceptions that could have caused an internal
| failure.
| BrandoElFollito wrote:
| > You don't know which exceptions will be raised in advance.
|
| > Most of exceptions are unrecoverable (that's why they are
| called exceptions), the best course of action is to crash,
| which happens by default.
|
| I prefer to propagate such unknown exceptions to a top-level
| catch to clanly log that something happened.
|
| I usually have two types of exceptions: the ones I expect at
| some point (a HTTP call failing for some reason) that I may
| ("when I have time") group in "known exceptions we should not
| worry about" and log them as "informational", and exceptions I
| did not anticipate that I want to log as well, but with a
| critical level because they were, well, unexpected.
|
| So crashing right when they happen may not be the best
| strategy.
| sandinmyjoints wrote:
| I really wish English had different individual words for the
| concepts "known [problem] we should not worry about" and
| "[problems] I did not anticipate".
| cduzz wrote:
| warning (a not fatal thing happened; took too long)
|
| error (a fatal thing I was expecting happened; I couldn't
| reach the API)
|
| exception (a fatal thing I wasn't expecting happened; the
| runtime fault handler did a thing named XYZ)
| hbrn wrote:
| ERROR and WARNING log levels.
|
| Everything goes to ERROR by default, some errors get
| downgraded to WARNING when you confirm they are not
| important.
|
| Send ERRORs to Sentry (or whatever you use) and deal with
| them immediately. Send WARNINGs to your favorite
| centralized logging solution and deal with them when
| there's too many.
| [deleted]
| greatpostman wrote:
| Like others have said, in theory this is great. In reality, I
| never see custom exceptions being handled differently than
| whatever exception was wrapped. And I have worked on some large
| distributed systems where failure is common. For new engineers
| these custom exceptions add abstract complexity and exception
| class hierarchy into a code base when it really isn't needed.
| IshKebab wrote:
| I disagree. I have seen plenty of code that is forced to match
| on the text of an exception because it uses a too-generic type.
|
| That said, I still wouldn't use a custom exception type in most
| languages simply because it's so tedious for a small pay-off.
| It's one of those things that you _should_ do, but isn 't
| really worth the hassle. Like putting alt text on HTML images.
|
| Do any languages with exceptions let you define new exception
| types at the throw site?
| Psyonic wrote:
| I don't see how an exception type defined at the throw site
| would be significantly different than using the text of the
| exception to convey the type. Callers wouldn't know to expect
| it either way, so how would they be able to handle it
| effectively?
| cratermoon wrote:
| This idea can also be explored in the Go programming language. Go
| has an error type, not exceptions, but error checking famously
| can be rather verbose.
|
| Two cases to consider come to mind. First, the common pattern
| result, err := SomeFunc() if err != nil {
| return err }
|
| Here the code is just passing along the error to the caller,
| unchanged.
|
| Second, signaling errors ab initio result := //
| some calculation or behavior if result != expected
| return fmt.Errorf("An error happened. Expected %v, go %v",
| expected, result)
|
| In the first case, the discussions around whether or not to throw
| custom exceptions applies analogously: should you wrap the error
| or not?
|
| The second case, I argue, is always wrong. The error is "stringly
| typed", and can be examined and read by a person, but that's it.
| The correct way is to define an error type meaningful for the
| context. Errors in Go are type implementing the error interface
| type error interface { Error() string }
|
| therefore, an error should be a type relevant or the context. A
| useful starting point looks something like type
| DomainError struct { Code DomainErrorCode
| Message string Details []DomainType }
| func (d DomainError) Error() string { return
| Message }
|
| then code can look like // some work if
| result != expected { return DomainError {
| Status: AnErrorCode Message:
| fmt.Sprintf("Error %v. Expected %v, go %v", AnErrorCode expected,
| result) Details: []DomainType{ expected,
| result } } }
|
| And the caller gets back a type providing useful information.
| kitd wrote:
| IME, returning a "stringly-typed" error is only wrong if you
| don't provide enough context in the returned error message. You
| can describe what you were doing as an error message and pass
| it back up the stack, each layer adding their own context, ie
| what they were trying to do to what. At the top you (should)
| get a pretty complete picture of what went wrong and why. I
| have found this reasonable and proportionate for most
| scenarios.
| cratermoon wrote:
| That's all well and good if all you want to do it propagate
| the error up the call stack until some code prints it, but
| that's not really _handling_ the error, any more than if err
| != nil { return error } is. If the calling code needs to
| behave differently depending on the details of the error, the
| worst way to do that would be examine the message string. A
| robust system is going to respond very differently if, for
| example, the error is one that indicates retrying is
| worthwhile vs an error indicating that no amount of retrying
| will ever succeed.
| tsimionescu wrote:
| The whole DomainError thing only makes sense if you expect
| someone to handle this error (and by handle, I mean doing
| something other than logging and aborting some operation), or
| if you are writing a very generic library. Otherwise, it is
| wasted time and extra complexity that makes the code harder to
| read.
| cratermoon wrote:
| Yes, that's true, just as it is with exceptions in the
| original article. However, if you are writing the code and
| it's callers, if you don't want to handle the error, why even
| have it? Just log the results there and move on.
| gwd wrote:
| > However, if you are writing the code and it's callers, if
| you don't want to handle the error, why even have it? Just
| log the results there and move on.
|
| Well at very least you need the caller to know that the
| operation didn't succeed. And if the failure is deep in the
| call stack, you may not have enough information to log a
| meaningful error. Returning errors and wrapping them with a
| description as you pass them up the stack means you can
| have a single meaningful log message at the "top level"
| (wherever the buck stops).
| maria2 wrote:
| It depends on how much context is needed. Imagine you have a
| Go library that parses JavaScript. Parsing can return an
| error, and it's very helpful to know information like the
| line and column number. So you might have an error like this:
| type ParseError struct { Line int Col
| int Message string }
|
| The user of your library isn't expected to handle this error
| explicitly, but when they print the output, they can see
| exactly where the issue was.
| teddyh wrote:
| Yes. The Python modules I write generally have:
| class Error(Exception): pass class
| FooError(Error): ...
|
| and then, in the generic case: try:
| failing_external_function() except Exception as e:
| raise Error(*e.args) from e
|
| (And if the code encounters a foo situation, it explicitly raises
| FooError, possibly with extra parameters, etc.)
|
| It is _really_ annoying to have to catch _all_ of, say,
| socket.error, SSL errors, FileNotFound, etc. etc. in _every_ call
| to some function in a module, if you use that module extensively.
| It's much easier to just catch module.Error and be done with it.
| If you need to handle foo errors specially, you catch FooError.
| hbrn wrote:
| What happens if function from another module calls
| failing_external_function()? Will you wrap it in
| another_module.Error?
|
| This sounds like viral boilerplate: any function that can fail
| forces all other functions in the stack trace to be wrapped.
| Seems quite unpythonic, the whole point of exceptions is to
| avoid this boilerplate by propagating.
|
| Are you just writing boilerplate to assign exceptions to
| modules? But this information is already present in the stack
| trace, what is the point? It's too generic to actually handle
| exceptions, and too redundant to provide debugging value.
___________________________________________________________________
(page generated 2022-12-19 23:01 UTC)