Propagate Java throwables during load without wrapping#8668
Propagate Java throwables during load without wrapping#8668headius merged 1 commit intojruby:10-devfrom
Conversation
|
This should ideally be safe, since a Java exception during load is unlikely to be recoverable and LoadError at worst might trigger users to try the load again. However, if someone is expecting a Java error under some circumstances, and using the resulting LoadError to re-try, this will break that logic. We have talked for years about not wrapping load-time Java exceptions at all, since as LoadError it masked important data. That is not changed here; we just switch to using RuntimeError and still hide the Java exception. This fix is primarily intended to increase the severity level of such raised errors, ensuring that they won't be mistaken for recoverable LoadError. |
d9fdd4c to
78e46c0
Compare
78e46c0 to
965e3b2
Compare
|
After some discussion we have decided to go all the way to propagating these throwables without any wrapper at all. This improves debugging (the original throwable is always visible so we don't need to pass One exception to this is IOException, which will now propagate as an appropriate IOError. This is how we handle it elsewhere, and code expecting IO failures during load to propagate as an Errno will now work properly. |
We have long re-raised Java exceptions during load as LoadError, but this indicates that the problem was with booting up Ruby code or libraries rather than a more fatal Java error. Also, because we raise LoadError, RubyGems will think that it might be able to help by re-activating the gem in question and re-requiring, which can lead to confusing double loads and duplicate constant definitions. We considered wrapping with a different exception, like RuntimeError, but that doesn't fix the problem of hiding the original Java throwable in a non-obvious way. Users can specify the -d flag to print out throwable stack traces during load, but most don't know to do it and it is often inconvenient. This patch makes the following changes for Java throwables raised during load/require: * If the throwable is an IOException, propagate it as the appropriate Ruby error, as we do for other IOException propagating out of Java code. * All other non-Ruby Java throwables get propagated as-is without any wrapper. * Clean up some redundant cathes that are unnecessary now: * MainExitException is Unrescuable and will propagate. * CatchThrow ditto Relates to duplicate constant definitions during load of gem sources as described in the comment below: sparklemotion/nokogiri#3452 (comment)
965e3b2 to
7925b40
Compare
Propagate Java throwables during load
We have long re-raised Java exceptions during load as LoadError,but this indicates that the problem was with booting up Ruby code or libraries rather than a more fatal Java error.
Also, because we raise LoadError, RubyGems will think that it might be able to help by re-activating the gem in question and re-requiring, which can lead to confusing double loads and duplicate constant definitions.
We considered wrapping with a different exception, like RuntimeError, but that doesn't fix the problem of hiding the
original Java throwable in a non-obvious way. Users can specify the -d flag to print out throwable stack traces during load, but most don't know to do it and it is often inconvenient.
This patch makes the following changes for Java throwables raised during load/require:
Relates to duplicate constant definitions during load of gem sources as described in the comment below:
sparklemotion/nokogiri#3452 (comment)