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

WebSocketSession#close never emit when using ReactorHttpServer [SPR-17306] #21839

Closed
spring-issuemaster opened this issue Sep 26, 2018 · 7 comments
Closed
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Sep 26, 2018

Sola opened SPR-17306 and commented

Given the following handler snippet,

@Override
public Mono<Void> handle(WebSocketSession session) {
  return session.send(Flux.error(new Throwable())
      .onErrorResume(e -> session.close(CloseStatus.GOING_AWAY))
      .then(Mono.empty()));
}

when using ReactorHttpServer, the Mono from session#close never emit.
!image-2018-09-27-05-12-22-853.png|thumbnail!


Affects: 5.0.9

Attachments:

Referenced from: commits 42b7c5a, b462ca2, bf4d00c, 1320fed

Backported to: 5.0.11

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 1, 2018

Rossen Stoyanchev commented

I can confirm the issue and investigating. In the mean time as a workaround, make the "send" and the "close" sequential (and not nested), e.g.:

@Override
public Mono<Void> handle(WebSocketSession session) {

  Flux<WebSocketMessage> output = Flux.error(new Throwable())
      .onErrorResume(e -> Flux.empty())
      .cast(WebSocketMessage.class);

  return session.send(output)
    .then(Mono.defer(() -> session.close(CloseStatus.GOING_AWAY)));
}
@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 2, 2018

Rossen Stoyanchev commented

I've confirmed the issue with Reactor Netty only, and created reactor/reactor-netty#444.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 2, 2018

Rossen Stoyanchev commented

The actual fix should come from Reactor but I'm turning this into a task and keeping it open and scheduled for the next set of releases. Depending on the outcome of the Reactor Netty ticket, we may have some follow-up to do here.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 12, 2018

Rossen Stoyanchev commented

Moving to 5.x.backlog while waiting for the Reactor Netty fix.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 12, 2018

Sola commented

According to your workaround, what if I have to send the corresponding CloseStatus base on what Exception I got?

For example:

userPrincipal.flatMap { principal ->
    userService.fetchUserInfo(principal)
        .onErrorResume { e ->
            logger.error("Failed to fetch user's info", e)
            close(CloseStatus.SERVER_ERROR.withReason(e.message!!)).then(Mono.empty())
        }
}.switchIfEmpty(close(CloseStatus.NOT_ACCEPTABLE.withReason("Unauthorized.")).then(Mono.empty()))
@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 19, 2018

Rossen Stoyanchev commented

SolaDev, a fix is coming to Reactor Netty. 

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 23, 2018

Rossen Stoyanchev commented

This is now fixed in Reactor Netty and will be in 0.7.11 (Bismuth SR13) and 0.8.2 (Californium SR2). There are corresponding changes in the Spring Framework to make use of the new sendClose available in NettyWebsocketOutbound.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.