Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it assumed that CompositeException is eligible to modify the cause of exceptions passed in constructor of CompositeException? #6747

Closed
zubchenok opened this issue Dec 5, 2019 · 5 comments · Fixed by #6748

Comments

@zubchenok
Copy link

zubchenok commented Dec 5, 2019

Since RxJava modifies exceptions during call of getCause method of CompositeException instance and connect these exceptions in chains we have finally millions of exceptions and quickly getting out of memory.

    @Test
    public void compositeExceptionIssue() {
        Single
            .just(new Throwable("ROOT ERROR"))
            .flatMapCompletable(rootError -> Observable
                .range(1, 10)
                .flatMapCompletable(testNumber -> Completable
                    .mergeArrayDelayError(
                        Completable.error(new RuntimeException("Test#" + testNumber + "A", rootError)),
                        Completable.error(new RuntimeException("Test#" + testNumber + "B", rootError))
                    )
                    .doOnError(Throwable::getCause)
                    .onErrorComplete()
                )
                .doOnComplete(() -> {
                    rootError.printStackTrace();
                })
            )
            .blockingAwait();
    }

This simple test demonstrates that cause of rootError throwable is changed.

@akarnokd
Copy link
Member

akarnokd commented Dec 5, 2019

Do you mean those "Caused by: java.lang.RuntimeException: Duplicate found in causal chain so cropping to prevent loop" listings?

@akarnokd
Copy link
Member

akarnokd commented Dec 5, 2019

The getCause looks needlessly convoluted. I presume it was implemented that way that IDEs show some usable stacktraces of the individual inner exceptions. Java 7's addSuppressed is better in this regard but we can't use it just yet.

I can probably have such duplicate indicator show up at most once.

@zubchenok
Copy link
Author

I mean that original exception is actually chained with other exceptions. rootError.getCause()!=null while expected to be null (in my option).

@akarnokd
Copy link
Member

akarnokd commented Dec 5, 2019

I see. To resolve it, we'd have to remove the custom getCause implementation otherwise it will keep touching all sorts of inner exceptions. We could also probably fake a StackTraceElement array containing all inner exceptions too.

@akarnokd
Copy link
Member

akarnokd commented Dec 6, 2019

Thanks for reporting. This is a shortcoming of the 1.x and 2.x composite design and I'm afraid we can only resolve this for 3.x because it is a breaking change in the structure of the composite verified by unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants