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

Add coroutine variant of WebExceptionHandler #32931

Conversation

earlgrey02
Copy link
Contributor

As in the case of CoWebFilter, we added CoWebExceptionHandler, a coroutine variant of WebExceptionHandler used for global exception handling in Spring WebFlux.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 1, 2024
@jhoeller jhoeller added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jun 3, 2024
@snicoll snicoll added theme: kotlin An issue related to Kotlin support labels Jun 4, 2024
@sdeleuze
Copy link
Contributor

sdeleuze commented Jun 6, 2024

We need to be careful about not going too far in terms of Coroutines duplications of Reactive contrats, but I think I am in favor of that one for 2 reaons:

  • That seems indeed pretty close to CoWebFilter use cases which is pretty popular among Kotlin developers I think
  • This is not yet reflected in the PR, but CoWebExceptionHandler could introduced the same kind of Coroutines context management as in CoWebFilter.

@rstoyanchev Would you be ok for that?

@earlgrey02 What about refining your PR to provide Coroutines context management like CoWebFilter does?

@sdeleuze sdeleuze self-assigned this Jun 6, 2024
@rstoyanchev
Copy link
Contributor

If CoWebFilter exists then yes a CoWebExceptionHandler next to it makes sense.

@earlgrey02
Copy link
Contributor Author

earlgrey02 commented Jun 6, 2024

We need to be careful about not going too far in terms of Coroutines duplications of Reactive contrats, but I think I am in favor of that one for 2 reaons:

  • That seems indeed pretty close to CoWebFilter use cases which is pretty popular among Kotlin developers I think
  • This is not yet reflected in the PR, but CoWebExceptionHandler could introduced the same kind of Coroutines context management as in CoWebFilter.

@rstoyanchev Would you be ok for that?

@earlgrey02 What about refining your PR to provide Coroutines context management like CoWebFilter does?

Instead of creating a separate COROUTINE_CONTEXT_ATTRIBUTE for CoWebExceptionHandler, I reused the COROUTINE_CONTEXT_ATTRIBUTE used in the existing CoWebFilter and handler(CoroutineContextAwareHandlerFunction).

@sdeleuze sdeleuze removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 11, 2024
@sdeleuze sdeleuze added this to the 6.2.0-M4 milestone Jun 11, 2024
sdeleuze added a commit to sdeleuze/spring-framework that referenced this pull request Jun 11, 2024
@sdeleuze sdeleuze closed this in 4c73747 Jun 11, 2024
@sdeleuze sdeleuze changed the title Add coroutine variant of WebExceptionHandler Add coroutine variant of WebExceptionHandler Jun 11, 2024
@sdeleuze
Copy link
Contributor

Merged and polished, thanks for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: kotlin An issue related to Kotlin support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants