AssemblyStackTraceException doesn't account for unknown causes

[pyricau]

AssemblyStackTraceException#attachTo method does this:

    /**
     * Finds an empty cause slot and assigns itself to it.
     * @param exception the exception to start from
     */
    public void attachTo(Throwable exception) {
        Set<Throwable> memory = new HashSet<Throwable>();

        for (;;) {
            if (exception.getCause() == null) {
                exception.initCause(this);
                return;
            }

            exception = exception.getCause();
            if (!memory.add(exception)) {
                // in case we run into a cycle, give up and report this to the hooks
                RxJavaHooks.onError(this);
                return;
            }
        }
    }

This assumes that there are two types of exceptions: either it doesn't have a cause and we can set one, or there is a cycle in the chain and it bails.

However, if you look at the JDK Throwable implementation, you'll see this:

public class Throwable  {

    /**
     * The throwable that caused this throwable to get thrown, or null if this
     * throwable was not caused by another throwable, or if the causative
     * throwable is unknown.  If this field is equal to this throwable itself,
     * it indicates that the cause of this throwable has not yet been
     * initialized.
     */
    private Throwable cause = this;

   public Throwable(String message, Throwable cause) {
        fillInStackTrace();
        detailMessage = message;
        this.cause = cause;
    }

    /**
     * Initializes the <i>cause</i> of this throwable to the specified value.
     * (The cause is the throwable that caused this throwable to get thrown.)
     *
     * <p>This method can be called at most once.  It is generally called from
     * within the constructor, or immediately after creating the
     * throwable.  If this throwable was created
     * with {@link #Throwable(Throwable)} or
     * {@link #Throwable(String,Throwable)}, this method cannot be called
     * even once.
     *
     * @param  cause the cause (which is saved for later retrieval by the
     *         {@link #getCause()} method).  (A {@code null} value is
     *         permitted, and indicates that the cause is nonexistent or
     *         unknown.)
     * @return  a reference to this {@code Throwable} instance.
     * @throws IllegalArgumentException if {@code cause} is this
     *         throwable.  (A throwable cannot be its own cause.)
     * @throws IllegalStateException if this throwable was
     *         created with {@link #Throwable(Throwable)} or
     *         {@link #Throwable(String,Throwable)}, or this method has already
     *         been called on this throwable.
     */
    public synchronized Throwable initCause(Throwable cause) {
        if (this.cause != this)
            throw new IllegalStateException("Can't overwrite cause with " +
                                            Objects.toString(cause, "a null"), this);
        if (cause == this)
            throw new IllegalArgumentException("Self-causation not permitted", this);
        this.cause = cause;
        return this;
    }

So, if you do new Throwable("", null), then the cause is set, and it is null. Throwable.getCause() will return null, but Throwable.initCause() will throw. And we just saw that attachTo does this:

            if (exception.getCause() == null) {
                exception.initCause(this);
                return;
            }

This was introduced when fixing #4212 .

This is going to be tricky to fix, because there is no easy to know "this exception does have a cause and it is set to null".

It's ugly, but we could catch IllegalStateException when calling initCause(). And even then, I'm not sure what we should do. Reporting to RxJavaHooks.onError(); seems wrong, this isn't necessarily an incorrect exception.

FYI, Retrofit 1 has this exact issue (RetrofitError)

[pyricau]

Or... well, this is going to be funky, but I guess we could also recreate an entirely new exception that has exactly the same stacktrace but an unset cause.

[akarnokd]

I don't see any other way than catcing ISE and signalling to the hooks. Care to submit a PR?

[pyricau]

on it.

[akarnokd]

Closing via #4740.