[Feature request] Opt-in propagation of transformer exceptions

[Marcono1234]

Problem

Currently when a transformer throws an exception, Budy Buddy logs an error but execution continues. This is due to JDK's TransformerManager silently ignoring exceptions (JDK-8336165).

This can make troubleshooting difficult, because your application may behave in unexpected ways and you then have to go through the console log to find out that class transformation failed.

Suggested feature

Because it is not clear when / if that JDK bug is fixed, it would be great if Byte Buddy's AgentBuilder had an opt-in method for propagating transformer exceptions.

When enabled, Byte Buddy's AgentBuilder.Default#doInstall would then wrap the ClassFileTransformer to catch any thrown throwables, store them somewhere but also rethrow them (to account for the case that the JDK exception handling is improved in the future). Once doInstall finishes, it could then check if any throwables were collected and rethrow them.

Note: This might lead to duplicated output in the console log, but I personally would not mind that.

Workaround

Define a custom AgentBuilder.Listener (possibly using Listener.Compound to combine it with an existing listener) whose onError method collects all throwables, and then after calling AgentBuilder#installOn check if your custom listener collected any throwables and rethrow them.

Possibly also do this for AgentBuilder.InstallationListener onWarmupError.

But I am not sure if that covers all cases.

[Marcono1234]

Regarding InstallationListener's onWarmupError, I am not completely sure how it behaves currently. But it might be good to handle the throwable in some way or fail fast for it, and not silently discard it.

---

Maybe in a future Byte Buddy version it would make sense to make such a behavior opt-out instead of opt-in to make troubleshooting for users easier? Assuming that use cases where a transformer intentionally throws an exception are rare, and should be a deliberate decision.

What do you think?

[raphw]

You can already return a Throwable from an InstallationListener, or throw an exception from onError. This error will be propagated to the invocation.

As for the transformation, the current behavior wants to assure that load time and dynamic transformation showcases the same behavior where the exception is swallowed. As I understand it, the exception is however already propagated:

throw new IllegalStateException("Failed transformation of " + name, throwable);

[Marcono1234]

You can already return a Throwable from an InstallationListener, or throw an exception from onError. This error will be propagated to the invocation.

But that does not seem to cover transformation errors? For example if I use an @Advice.Argument with wrong parameter index then due to JDK's TransformerManager behavior the InstallationListener#onError is not reached.

Though even if it was reached you would have to additionally specify AgentBuilder.RedefinitionStrategy.Listener.ErrorEscalating, otherwise Byte Buddy would not propagate the error either?

---

Here is a somewhat minimal reproducer (JDK 17, Byte Buddy 1.17.5):

import net.bytebuddy.agent.ByteBuddyAgent;
import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.matcher.ElementMatchers;

import static net.bytebuddy.matcher.ElementMatchers.hasMethodName;
import static net.bytebuddy.matcher.ElementMatchers.is;

public class ByteBuddyTest {
    public static void main(String[] args) {
        Class<?> transformedClass = System.class;

        new AgentBuilder.Default()
            .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
            .with(AgentBuilder.RedefinitionStrategy.Listener.ErrorEscalating.FAIL_FAST)
            .disableClassFormatChanges()
            .ignore(ElementMatchers.nameStartsWith("net.bytebuddy."))
            .with(AgentBuilder.InstallationListener.StreamWriting.toSystemOut())
            .with(new AgentBuilder.Listener.Filtering(
                ElementMatchers.anyOf(transformedClass.getName()),
                AgentBuilder.Listener.StreamWriting.toSystemOut()
            ))
            .type(is(transformedClass))
            .transform((builder, type, classLoader, module, protectionDomain) -> {
                return builder.visit(Advice.to(BrokenAdvice.class).on(hasMethodName("lineSeparator")));
            })
            .installOn(ByteBuddyAgent.install());

    }

    public static class BrokenAdvice {
        @Advice.OnMethodEnter
        public static void onEnter(@Advice.Argument(1000) int invalid) {
        }
    }
}

Transformation fails, but the error is only printed to the console and AgentBuilder#installOn returns without exception.

[raphw]

Byte Buddy is implicitly calling the JDKs retransformClasses0 where the exception is suppressed. You could, of course, manage some state and register a listener that adds the exception to it. But this will not allow aborting the current batch either.

Therefore, I find this rather unreliable and do not plan to add first-level support in Byte Buddy for it and rather wait if the JDK adds some mechanism for this in the future.

[Marcono1234]

Yes registering a Listener which collects the errors and throws them after AgentBuilder#installOn returns is what I am currently doing. At least for my use case that is already better than not throwing an exception, even if it is not fail-fast.

---

Therefore, I find this rather unreliable and do not plan to add first-level support in Byte Buddy for it and rather wait if the JDK adds some mechanism for this in the future.

Fair point, feel free to close this issue then if you want. And thanks nonetheless for your feedback!

[raphw]

I'd suggest a ThreadLocal for collecting the errors, even though the JVM does not specify to avoid concurrent class transformation.