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

Clarify handling exceptions during async events #599

Closed
manovotn opened this issue Mar 31, 2022 · 5 comments · Fixed by #601
Closed

Clarify handling exceptions during async events #599

manovotn opened this issue Mar 31, 2022 · 5 comments · Fixed by #601
Labels
spec-clarification An issue requesting clarification in the specification

Comments

@manovotn
Copy link
Contributor

The specification has a chapter on this but it is IMO not entirely clear because it only covers the case where more than one exception is thrown.

The TCK likewise only covers such scenario, see https://github.com/eclipse-ee4j/cdi-tck/tree/master/impl/src/main/java/org/jboss/cdi/tck/tests/event/observer/async/handlingExceptions

We should change the chapter title to something like Handling exceptions thrown during an asynchronous event and in the text we need to either explicitly state that the cause is always CompletionException or just mention what happens in case there is only one exception.

Afterwards, we should add a TCK test for it.

Note that this was originally brought up in Quarkus issue - quarkusio/quarkus#24646

@Ladicek
Copy link
Contributor

Ladicek commented Mar 31, 2022

I agree that we should get rid of the word "multiple" in the name of the section, but "CompletionException contains all exceptions thrown by observers as suppressed exceptions" is IMHO completely clear in what should happen if only one exception occurs.

@manovotn
Copy link
Contributor Author

@Ladicek I see it the same way but in the least we should add a TCK test that asserts it. I have create all the needed issues so it hopefully won't be forgotten :)

@Ladicek
Copy link
Contributor

Ladicek commented Mar 31, 2022

Yea totally agree about the TCK.

@Emily-Jiang
Copy link
Contributor

Does this require another respin of the release? If yes, I will need to withdraw the ballot again. please let me asap.

@starksm64
Copy link
Contributor

No as this is not a new issue. It is simply additional test coverage that can be rolled into the next TCK update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec-clarification An issue requesting clarification in the specification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants