JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.65k stars 5.48k forks source link

rationalize uses of error() and throw() #5694

Closed JeffBezanson closed 9 years ago

JeffBezanson commented 10 years ago

Currently all of these do the same thing:

error(BoundsError)
error(BoundsError())
throw(BoundsError())

We're not consistent about which of these should be used. It's unnecessarily confusing. Some background:

If we were going to use throw in the control-flow sense, it would probably need 2 arguments (throw(label, value)), so we don't necessarily need 2 names to disambiguate.

My preferred idiom is throw(SomeException()) since it clearly makes an exception object and then throws it. The only downside is verbosity.

Wrapping a string in an ErrorException obviously doesn't add any interesting information, so maybe throw("message") should be considered acceptable. Then error could be an alias or deprecation for throw.

ssfrr commented 10 years ago

It may be worth looking at Lumberjack.jl, which seems like a very sane approach to logging (though I'm not sold on the cutesy names). I like the approach that debug(), info(), warning(), and error() all take a string and append it to the log at the given level. error() also throws an ErrorException with the given string (basically like Base.error). In my mind that gives a more clear conceptual framework to the relationship between error() and throw().

As a side note - When I started using the logging module in python instead of scattering my code with prints it was a huge win. Being able to change the log level of the program rather then commenting in and out log messages all the time is something people overlook the value of. Not sure what the criteria are for when things get pulled into Base, but i think that encouraging this sort of logging is a good thing and has basically zero overhead when writing code.

jakebolewski commented 10 years ago

+1 for throw(SomeException("msg")). I feel it is much more explict and easier to understand what is going on. I hope that it also encourages people to write more descriptive error objects. Sometimes the only way you can tell what went wrong is to match against a ErrorException's message string which is annoying. I feel that we should follow Python's lead here, error() seems like a vestige from matlab.

jiahao commented 10 years ago

fwiw, I've put in some effort to systematize the linalg codebase to use throw(SomeException(...)) except when the exception doesn't currently take a message field when I think it would be useful to.

johnmyleswhite commented 10 years ago

Another +1 for throw.

I also like the idea of pulling a logging framework into Base once the frameworks become more mature.

JeffBezanson commented 10 years ago

I find errors are often hard to classify. For example I just noticed

    m==n || throw(DimensionMismatch("multiplicative identity defined only for square matrices"))

While most DimensionMismatches arise for two things with different shapes that should be the same, this one is for non-squareness. In still other cases the exact shape doesn't have to match, but the number of elements does. It's not clear if there should be a NotSquare exception type. These kinds of concerns add lots of cognitive overhead when trying to be diligent and handle all error cases. I think this is why people (myself included) like to fall back on the ease-of-use of error().

I'm not sure what the solution is, but there should probably be relatively few exception types so you don't have to think too much about which one to use.

jiahao commented 10 years ago

I'll hazard a guess here. I think the current error vs Exception situation is unsatisfactory is because a) we don't really do much with Exceptions yet, and b) few of the existing Exceptions encapsulate information that is actually that useful for a caller to catch and act upon; quite a few Exception types do little other than wrap an error message in a string, and aren't really that much more useful than error(somestring). Sometimes the subtype of Exception is semantically useful: LinAlg itself contains three Exceptions which do nothing but wrap error codes from LAPACK, ARPACK and CHOLMOD. However, the current situation does presume that the user is cogent enough to realize that the errors come from the underlying libraries and will look up the error codes themselves.

To carry over an idea from #4836, I wanted DimensionMismatch to display a standardized error message. Ideally the process throwing the exception could call some constructor that was given the identifiers of the variables responsible, and one would create a DimensionMismatch that would capture some semantic information about the sizes of the variables passed to the constructor, so that in principle, an error handler could work out what happened, and if the decision is made to output to console, the error message would be standardized, e.g. "Dimensions of A (20,15) and B (14, 10) are incompatible for [whatever]".

jiahao commented 10 years ago

If DimensionMismatch applied to a single object is wrong, then maybe this is an instance of the square-rectangle problem.

jakebolewski commented 10 years ago

I agree that error messages are hard to classify, and having lots of error types is sometimes just as confusing. But I think your example highlights what I was trying to get at before. If you were consuming the above code and it threw an error, isn't DimensionMismatch much more informative about what went wrong than ErrorException even though it is an imperfect description of the problem? You could include an informative string in the error message, but what if the informative string included (ideally) specific information as jiaho stated. Then if you want to act on the error message you have to resort to pattern matching against the error string to figure out what went wrong. I'm all for simplicity, but Python for instance has ~50 exception objects in the core language alone.

JeffBezanson commented 10 years ago

Ultimately I agree we will have to bite the bullet and remove error in favor of throwing specific exception types.

nalimilan commented 10 years ago

One approach would be to recommend using very specific exceptions, and make them inherit from more general ones when applicable. For example, NotSquare would inherit from DimensionMismatch, which allows callers to choose the most appropriate level of specificity for their use case.

Ideally, I think every error message should be generated automatically by combining the exception type and arguments, just like suggested in @jiahao's comment https://github.com/JuliaLang/julia/issues/5694#issuecomment-34343173 above. This could even be enforced by only allowing error messages to be printed via arguments to a specific exception type.

kmsquire commented 10 years ago

Is there an issue already for catching specific exception types? A quick search didn't find one. Obviously, we can use isa to test the type, as is shown in the manual, but I would love to see something less verbose.

(Better non-dispatch-based pattern matching is something I'm still wanting in the core language... cf. https://github.com/kmsquire/Match.jl, #3146, #5410)

jakebolewski commented 9 years ago

The error(::Exception) methods have been deprecated in 0.4-dev in favor of throw, so this issue seems like it can be closed now.

hayd commented 9 years ago

To clarify, the consensus is that any error(...) calls in base should be made more specific?