except: should map to the more conservativeexcept CatchableError: instead of except Exception:

[timotheecour]

follow up for nim-lang/Nim#8237, and with the same principles ("Technically, assertion failures are not catchable."), except: should map to the more conservativeexcept CatchableError: instead of except Exception:

User can always explicitly set except Exception: (or even except Defect:depending on his use case) when he needs, eg in unittests.

when defined(case1):
  # prints ok3 (OK!)
  try:
    doAssert false, "basf"
  except CatchableError:
    echo "ok1"
  except Exception:
    echo "ok3"

when defined(case2):
  # BUG: prints ok1; should give `Error: unhandled exception:` and behave like `except CatchableError:` which is a saner default
  try:
    doAssert false, "basf"
  except:
    echo "ok4"

when defined(case3):
  # BUG: this gives a C error: timn_t44_try_except_throwable.c:444:3: error: expected expression
  try:
    doAssert false, "basf"
  except:
    echo "ok4"
  except Defect:
    echo "ok5"

note

• I remember someone mentioned a plan to deprecate except: but now that @Araq implemented CatchableError as root of all ethically catchable exceptions, it makes sense to keep except: , and give it the meaning of except CatchableError:; it's a sensible default.
• this was part of original RFC [RFC] change AssertError in exception hierarchy to allow simpler exception handling  Nim#8237 (albeit more like a footnote...); the hard part was already done; this is a small but logical improvement

[Araq]

I agree completely with this proposal and the reasoning behind it.

[LemonBoy]

I've implemented this, there are a few problems with some specific cases (beside the endless and boring churn work needed to adjust all the tests).

Foreign (js/cpp) exceptions cannot be caught anymore, let me explain this point with an excerpt from the test suite:

import json

try:
  discard parseJson"""{ invalid"""
except:
  echo "CAUGHT"

Right now the output is CAUGHT because we swallow the JS-native SyntaxError produced by the JSON module used under the hood. You can't really write except Exception and expect it to work since the exception handler knows that it's not dealing with a subclass of Exception and just raises it.

Moreover I noticed another problem, if a foreign (not a Exception subclass) is caught in the default exception handler all you get is a lousy segfault since it is treated as if it were a Nim one with all its fields and stuff.

Oh, and one of the Nim in Action examples has to be modified (protocol.nim) otherwise, don't know if that's frowned upon.

[metagn]

A solution to foreign exceptions/exceptions that dont extend one of CatchableError or Defect is the following:

try:
  something()
except Defect:
  raise
except:
  handleException()

I think this is how it should be regardless, the system module can't categorize every single exception type in existence. CatchableError and Defect should be convenience options, not rule with an iron fist. As for whether or not the compiler can automate this, Defect could have a {.uncatchable.} pragma, in line with separating the standard library from the compiler, and the compiler should track this for except: statements.

[Araq]

With --panics:on and the refined ideas about what to track finally implemented, I think this RFC is of little relevance. Ping @timotheecour

[timotheecour]

with --panics:on it makes no difference indeed, but it does with --panics:off (which is the default), and we should reserve the concise syntax to be the safest, while still allowing user to customize for his needs

proposal: except {.foreign.}:

• this should solve the foreign exception issue mentioned above
• makes the concise syntax except: be the safe one
• except CppException as e: works as it does today, and in fact everything that already works would keep working the same apart from except:; and except {.foreign.}: would be introduced
• getCurrentException() would become a CT error inside foreign branches (except {.foreign.}: and except SomeForeignException:)

try: something()
except: # same as `except CatchableError:`
	let e = getCurrentException() # same `e` as captured with `except CatchableError e:`
except CppException as e: # ok, catches a specific foreign exception
  discard
  #  let e2 = getCurrentException() # this should be CT error, since CppException is foreign (eg importcpp)
except AssertionError as e: # ok, catches an AssertionError (eg for testing or long running servers, or REPL's etc)
  discard
except Defect as e: # ok, catches all other Defects
  let e2 = getCurrentException() # ok, same as e
except {.foreign.}:
  discard
  # let e = getCurrentException() # this should give CT here in this except branch

note: this is partly related to nim-lang/Nim#14369 btw.

[cooldome]

I agree with @Araq it can be closed.
The following example continue to work as is

try:
  something()
except Exception:
  handle all Nim Exceptions 
except:
  handleNonNimExceptions()

Comments about getCurrentException() are irrelevant. It can be used outside of try/except blocks and there are cases where it is required outside of try/except blocks.

[timotheecour]

my point about getCurrentException was that it's guaranteed to be nil when used directly inside a except Foo block where Foo is foreign:

try: bar()
except Foo: # user is unaware this is a foreign exception
  let e = getCurrentException()
  echo e.msg # SIGSEGV

so that case could be made illegal at CT; however it wouldn't prevent other cases where getCurrentException is called inside a proc eg:

try: bar()
except Foo: # user is unaware this is a foreign exception
  fun()

proc fun() =
  let e = getCurrentException()
  echo e.msg # SIGSEGV

so maybe it's not worth the special case

Anyway, closing, it's sane enough currently.

[zah]

What is the rationale for rejecting this proposal?

The reason why except: should default to CatchableErrror is that inexperienced (or just tired) Nim developers tend to use it in their code as a synonym for "I don't care what kind of recoverable error happens here, I'll treat all errors in the same way". It's similar to writing if errno != 0 or if( SUCCEEDED(hr)) in C code.

But since Defects are not recoverable errors, stopping their propagation is almost never appropriate. In the rare situations where it is appropriate, the user can use an explicit except Exception: ... or except Defect: ....

I've personally fixed more than 20 places in our codebase and third party libraries when except: ... was used inappropriately (the author should have used except CatchableErrror).

[timotheecour]

re-opening since there's no consensus yet. @zah what do you think of except {.foreign.}: to handle the above mentioned problem of foreign exceptions?
this could be:

try: bar
except: # means `except CatchableError:`; preferred concise syntax is mapped to the safe, sane option
except AsstionDefect: # useful in some cases, eg test frameworks / servers that can't die / some repl implementations
except MyCppException: # 1 particular foreign exception
except {.foreign.}: # catch-all for foreign exceptions
except {.all.}: # catch-all, both nim Exception and foreign exception

[zah]

We can perhaps work-around the problem of foreign exceptions with some magical types such as CppException and JsException that are specialized on the codegen level.

One could argue that in JavaScript mode (and C++), the default could be CatchableError, JsException, but reasoning about this quickly gets complicated (The JavaScript hierarchy itself can benefit from a Defect/Error distinction)

[timotheecour]

We can perhaps work-around the problem of foreign exceptions with some magical types such as CppException and JsException that are specialized on the codegen level.

that could work too, not really sure it's simpler (implementation wise or usage wise) than the simple pragma {.foreign.}

not that, unlike for C++, the following works for js:

try {
} catch (e) { // catches all exceptions AND have a reference to it via `e`
}

whereas in C++ the best catch-all you can do is:

catch(...) // no reference `e` possible

so JsException doesn't have to be magic and can just be JsObject;
(and e isinstanceof RangeError etc can still be used by user code)

but I'd prefer to use ForeignException instead of CppException or JsException so that it works cross platform; it'd jus be:

when defined(js):
  type ForeignException = JsObject
elif defined(cpp):
  type ForeignException {.magic: "ForeignException".} = object
    # this type is special in C++ mode; eg can't use sizeof(ForeignException) etc

[alehander92]

ok, can't just except: include foreign exceptions as well by default? otherwise we expect the user to find out about a special pragma: this seems very bug-prone to me
EDIT: it seems this is what @zah is proposing, nvm

[dom96]

Isn't this proposal a little unintuitive? I can't think of a programmer that wouldn't expect except: to catch ALL exceptions. Non-catchable exceptions are exceptions too. So why make the semantics special here?

[ringabout]

Now catching bare exceptions gives a warning now. It can be closed.