Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate error("msg")? #21886

Closed
c42f opened this issue May 15, 2017 · 31 comments
Closed

Deprecate error("msg")? #21886

c42f opened this issue May 15, 2017 · 31 comments
Labels
error handling Handling of exceptions by Julia or the user
Milestone

Comments

@c42f
Copy link
Member

c42f commented May 15, 2017

I've just been in contact with a new user, who was understandably using throw("my message") rather than throwing an ErrorException via error().

Do we need the conceptual overhead of having both error and throw? Why not conceptually have the following definitions:

throw(msg::AbstractString) = throw(ErrorException(msg))
throw(e::Exception) = builtin_throw(e)

A somewhat related question - what do we gain by allowing people to throw things which aren't subtypes of Exception?

@andyferris
Copy link
Member

I've wondered if throw("...") would be better, also.

@cstjean
Copy link
Contributor

cstjean commented May 15, 2017

A somewhat related question - what do we gain by allowing people to throw things which aren't subtypes of Exception?

In Common Lisp, it's accepted to use throw to return values from a deeply nested context, and it's sometimes much more convenient than passing the computation result through many functions.

@c42f
Copy link
Member Author

c42f commented May 15, 2017

I remember seeing this proposed before, and after some digging I found the comment by @kmsquire #13515 (comment)

@ararslan
Copy link
Member

throw throws an Exception object. throw(::String) doesn't make sense conceptually; it would have to be throw(ErrorException(::String)). But if you don't have an appropriate error type to use, just saying error is much cleaner.

@c42f
Copy link
Member Author

c42f commented May 15, 2017

You can, in fact, currently throw a string without wrapping it in an ErrorException,

julia> typeof(try ; throw("asdf") ; catch ex ; ex end)
String

The observation here is that this probably doesn't make a whole heap of sense, so we have the opportunity to turn two verbs into one by just removing error. It seems like a good pun to me.

@yuyichao
Copy link
Contributor

I don't think we should try to guess what the user want. We are in general moving away from error so I'll be fine to deprecate that. Allowing throwing a string and not actually throwing it is encouraging bad coding style.

@cstjean
Copy link
Contributor

cstjean commented May 15, 2017

We are in general moving away from error so I'll be fine to deprecate that.

Why?

@yuyichao
Copy link
Contributor

It offers no information of what actually happens.
Usually the error can be replaced with ArgumentError (which roughly corresponds to pythong's ValueError) if it doesn't fit in any other category.
We don't have a good type hierarchy for exceptions yet but we still should not encourage throwing the most general error possible.

@c42f c42f changed the title Deprecate error("msg") -> throw("msg") ? Deprecate error("msg")? May 15, 2017
@c42f
Copy link
Member Author

c42f commented May 15, 2017

I'd be content to simply deprecate error, though I still prefer the option of overloading throw. This is no more "guessing what the user wants" than all the other uses of multiple dispatch in julia.

The argument that we should encourage users to throw more specific exception types holds more weight.

@yuyichao
Copy link
Contributor

This is no more "guessing what the user wants" than all the other uses of multiple dispatch in julia.

No. Multiple dispatches should not be used that way. Multiple dispatch is generally used to select the best implementation and not completely changing the behavior.

@c42f
Copy link
Member Author

c42f commented May 15, 2017

Multiple dispatch is generally used to select the best implementation and not completely changing the behavior.

What do we gain by allowing users to throw things which are not subtypes of Exception? If the answer is "nothing", we can also deprecate that, and there would be no conceptual ambiguity here.

I agree that this is a change of behavior, and the deprecation would need to be handled correctly in stages.

@yuyichao
Copy link
Contributor

there would be no conceptual ambiguity here.

There is. Instead of giving an error you want it to do something that shouldn't be encouraged.

@andyferris
Copy link
Member

andyferris commented May 16, 2017

So to recap, my own interpretation of the proposal is this:

  • deprecate error
  • ensure that throw always ends up throwing something which is a subtype of Exception (or more precisely, that catch always receives an Exception).
  • however, if the user passes something that is not a subtype of Exception to throw (perhaps just a String), it automatically gets wrapped in ErrorException, for convenience.

In the last point, I see this as more-or-less "sugar" (it's not literally syntax sugar; we could use dispatch to arrange this) so that in the cases you want to be lazy, you can. Now, I do understand that it's much better practice to use the corresponding Exception type - but you have to admit that there are times that you are writing a hacky script and want to do it quickly but you want some useful error message at some point, and writing throw(ErrorException("...")) is too much overhead in my opinion. And I've found we don't have good exceptions for a lot of situations.

The consequences are that this "lispy" style of breaking through multiple stack frames by throwing whatever data type you want might not work as now. However, to make that safe, I think it would be better practice to make your own Exception subtype to explicitly mark and carry that information.

@yuyichao
Copy link
Contributor

but you want some useful error message at some point, and writing throw(ErrorException("...")) is too much overhead in my opinion.

If people just want this, then we should keep error and clearly document that it shouldn't be used to show user errors. FWIW, for writing something quick, @assert is usually good enough. I usually only switch away from it if I want the code to be more permanent in which case I'll pick the right error type. (which would be ArgumentError most of the time since I'm just checking input ranges).

@yuyichao
Copy link
Contributor

And I only said I'm fine with deprecating error because it should generally not be used, not because it should be done with a worse API. For the confusion between error("") and throw("")'s current behavior, only allowing throw to throw Exception is the right thing to do. There's also nothing special about string since error also does implicit string convertion.

The lisp usage of exception as control flow should not be as much of a concern in julia since we don't usually have as deep nesting AFAICT.

@cstjean
Copy link
Contributor

cstjean commented May 16, 2017

If people just want this, then we should keep error and clearly document that it shouldn't be used to show user errors.

+1. Base and serious packages should have properly typed exceptions, but for personal code, it would be annoying to specify which category each error fits into, because 99% of the time I have no intent of ever catching them, and the error message is all the information I want.

@c42f
Copy link
Member Author

c42f commented May 16, 2017

And I only said I'm fine with deprecating error because it should generally not be used, not because it should be done with a worse API

Whether this is worse depends on your point of view. Throwing generic errors is more convenient and concise (good things), but less precise (arguably a bad thing). It's fine to discount convenience and concision as not worth the tradeoff here, but they are legitimate points in the design space, and julia is stylistically a very concise language.

Furthermore, when exceptions are used to signal something very exceptional (programmer error) rather than to direct control flow, the detailed exception type is much less important: you probably shouldn't be directing flow control based on the type anyway. I feel this is a distinct departure from popular languages such as java, python, and C++, where flow control based on exception type is supported as part of the language, and is perfectly routine.

The lisp usage of exception as control flow should not be as much of a concern in julia

Yes, I hope we never start using exceptions for control flow 👍

@nalimilan
Copy link
Member

Furthermore, when exceptions are used to signal something very exceptional (programmer error) rather than to direct control flow, the detailed exception type is much less important:

Why not use @assert for that? It looks like the existence of error encourages abusing exceptions.

@c42f
Copy link
Member Author

c42f commented May 16, 2017

Why not use @assert for that

This is quite a good point, I'd forgotten that it allowed arbitrary descriptive text.

@c42f
Copy link
Member Author

c42f commented May 16, 2017

So what do people think of the following plan:

  • Deprecate error() in favor of @assert and throwing explicitly typed exceptions.
  • Disallow the throwing of arbitrary objects from throw - only allow subtypes of Exception.

@andyferris
Copy link
Member

I didn't know @assert could have a custom message - that makes it more useful! Not sure that it replaces every use of error, but probably the majority.

@kshyatt
Copy link
Contributor

kshyatt commented May 16, 2017

@andyferris sounds like something that should be more prominent in the manual, then, if someone wants to open a PR...

@martinholters
Copy link
Member

For me, @assert and error() have somewhat different objectives:

  • @assert says: Unless I messed up my code, this should hold; if it doesn't, bail out with an error descriptive enough to figure out what the problem was instead of continuing, producing a more hard-to-backtrack error later on.
  • error() says: I know this might occasionally go wrong (due to bad input, state, moon phase, whatever), but I'm too lazy to figure out the most appropriate Exception to throw here.

I see the merit in discouraging the use of error() (although it might be absolutely ok for your hacky script), but I don't think replacing it with @assert without further thought (because the deprecation told me so) is a good idea.

@c42f
Copy link
Member Author

c42f commented May 17, 2017

So, we're almost back to the start of this discussion.

  • throw allows beginners to throw somewhat nonsensical things (not subtypes of exception) by mistake.
  • On aesthetic grounds it seems unfortunate to need a special error() verb just for throwing a blessed exception type, especially one that's somewhat discouraged.

Does anyone have ideas about how to solve these, without loosing the concision of error()? For now, I'm all out, I think.

I suppose a survey of the way error() is used in packages would be useful at this stage.

@KristofferC
Copy link
Member

I use error when I feel that this error is unlikely enough to happen for me to not take the time to write something like throw(ArugmentError("...")). or when I don't know what actual exception fits best.

@martinholters
Copy link
Member

martinholters commented May 17, 2017

I think there are two related but separate issues here:

  1. Confusion of error("...") vs. throw("..."). Could be resolved by going with the OPs proposal or by limiting throw to take Exceptions.
  2. Freeing up error (to be used for logging). Could be resolved by going with the OPs proposal.

Concerning 2, I wonder how common a "log error and throw exception" pattern vs. a "throw an exception and log an error where it is caught" pattern would be. If the former is more common, it might even make sense to go the other way round and allow error to take an Exception. (I have no opinion here, just mentioning this as an option to consider.)

@fredrikekre
Copy link
Member

[...] go the other way round and allow error to take an Exception

Thats basically what MATLAB does, i.e. error() accepts either a message or an exception+message

@c42f
Copy link
Member Author

c42f commented May 17, 2017

Freeing up error (to be used for logging)

@martinholters Hah! You've discovered my other (not so secret) motive :-) In that case I want to use @error, but it seems problematic to have both @error and error() mean different things.

I wonder how common a "log error and throw exception" pattern vs. a "throw an exception and log an error where it is caught" pattern would be

I've worked in code bases (in C++) using both of these patterns, and I far prefer the second one. The catch site is pretty much forced to log the error in either case because it doesn't know whether the thrower already did so. This results in double logging of errors all over the place which is somewhat annoying and confusing.

@s-celles
Copy link
Contributor

s-celles commented Jul 16, 2018

I don't think

throw(Exception("msg"))

is possible because Exception is ever used as base exception (I'd have named it BaseException like in Python... but that's an other story).

Maybe

throw(Error("msg"))

with

Error(msg) = ErrorException(msg)

will be nice but I don't think there is a general consensus for this...

Maybe we could first deprecate use of error as function and suggest changing code from

error(msg)

to

throw(ErrorException(msg))

@ViralBShah ViralBShah added this to the 2.0 milestone Mar 11, 2022
@brenhinkeller brenhinkeller added the error handling Handling of exceptions by Julia or the user label Nov 21, 2022
@vtjnash
Copy link
Member

vtjnash commented Jan 31, 2024

At this point, I think we can say we are post 1.0 and not going to introduce a breaking change to how these work unnecessarily. error is a convenience constructor for throw(ErrorException(msg)). We could introduce Exception(msg...) = ErrorException(msg...) still, but it is unclear the value in that

@vtjnash vtjnash closed this as not planned Won't fix, can't repro, duplicate, stale Jan 31, 2024
@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Apr 19, 2024

Deprecating isn't a breaking change (only is when you actually remove the API)?! So we can do that in 1.x, if this is helpful. I think it has a runtime cost, but that needs not be the case? Do we need soft-deprecations like Python? Can't we have such and deprecations that only point to alternative in the REPL, but in a script is no-coast (on default options)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Handling of exceptions by Julia or the user
Projects
None yet
Development

No branches or pull requests