Skip to content

Propagate Java throwables during load without wrapping#8668

Merged
headius merged 1 commit intojruby:10-devfrom
headius:no_loaderror_wrapped_throwables
Mar 6, 2025
Merged

Propagate Java throwables during load without wrapping#8668
headius merged 1 commit intojruby:10-devfrom
headius:no_loaderror_wrapped_throwables

Conversation

@headius
Copy link
Member

@headius headius commented Mar 3, 2025

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:

  • 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)

@headius headius added this to the JRuby 10.0.0.0 milestone Mar 3, 2025
@headius
Copy link
Member Author

headius commented Mar 3, 2025

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.

@headius headius force-pushed the no_loaderror_wrapped_throwables branch from d9fdd4c to 78e46c0 Compare March 3, 2025 23:54
@headius headius force-pushed the no_loaderror_wrapped_throwables branch from 78e46c0 to 965e3b2 Compare March 6, 2025 21:10
@headius headius changed the title Raise Java errors during load as RuntimeError Propagate Java throwables during load without wrapping Mar 6, 2025
@headius
Copy link
Member Author

headius commented Mar 6, 2025

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 -d) and solves the original issue of RubyGems et al misinterpreting Java exceptions as something recoverable like a LoadError.

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)
@headius headius force-pushed the no_loaderror_wrapped_throwables branch from 965e3b2 to 7925b40 Compare March 6, 2025 21:15
@headius headius merged commit 66b4f3a into jruby:10-dev Mar 6, 2025
55 of 73 checks passed
@headius headius deleted the no_loaderror_wrapped_throwables branch March 6, 2025 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant