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

User API for websocket close with code and reason #2458

Open
johanandren opened this issue Mar 11, 2019 · 15 comments
Open

User API for websocket close with code and reason #2458

johanandren opened this issue Mar 11, 2019 · 15 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Identifies issues that the core team will likely not have time to work on t:core Issues related to the akka-http-core module t:websocket

Comments

@johanandren
Copy link
Member

A web socket connection can be closed with an application level status code (4000-4999) see https://tools.ietf.org/html/rfc6455#section-7.4 but we have no user level API for this in Akka HTTP.

@johanandren johanandren changed the title User API for web socket close with code and reason User API for web socket close with code Mar 11, 2019
@johanandren johanandren changed the title User API for web socket close with code User API for web socket close with code and reason Mar 11, 2019
@jrudolph jrudolph added help wanted Identifies issues that the core team will likely not have time to work on 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core Issues related to the akka-http-core module labels Mar 12, 2019
@jrudolph
Copy link
Contributor

Would be nice to support that.

Any idea how we put that in the API? We currently don't have a side-channel where we could put that information. I guess, we could either

  • create a special exception that can be thrown from the user handler to carry the code
  • create another subclass of ws.Message, CloseWithCode or something that would carry the information

@johanandren
Copy link
Member Author

Without grasping all consequences I think a subtype of Message sounds like a good idea. A bit unclear perhaps that it ends the stream but that is probably ok.

@mmacfadden
Copy link

I am 100% in favor of this. We definitely need this for our use case. We have been working around it for quite some time. For reference, I posted this on Stack Overflow almost a year ago with no answers:

https://stackoverflow.com/questions/50932959/how-to-get-websocket-close-code-from-akka-http

@raboof
Copy link
Contributor

raboof commented Jul 15, 2019

Good to have validation 'from the real world' here! Would you be interested in preparing a PR?

@stmaute
Copy link

stmaute commented Sep 23, 2019

So actually it isn't possible to close the websocket session and set a close code and reason?
Or is there any workaround for this?

@jrudolph jrudolph changed the title User API for web socket close with code and reason User API for websocket close with code and reason Mar 24, 2020
@prongs
Copy link

prongs commented Sep 23, 2020

Any updates on this? I have a use case where I want to accept only one WebSocket for one particular URL. If there's another request for the same URL, I want to reject that. Right now, I'm rejecting with HTTP 412 error code, but it seems that that error code is not available in javascript code.

This stackoverflow question describes a similar problem:

https://stackoverflow.com/questions/21762596/how-to-read-status-code-from-rejected-websocket-opening-handshake-with-javascrip

The recommendation from the accepted answer is to accept the websocket and immediately close it with an application-specific error code and error message.

I understand that it's not an implemented feature as of now, but is there any workaround available to achieve that?

@swsnr
Copy link

swsnr commented Mar 25, 2021

I'd also need this for a compliant implementation of a GraphQL over Websocket protocol; I am happy to submit a pull request, but I'd appreciate some direction about the proper way to implement this, @jrudolph

I tend to favour a custom exception. Adding a new message subtype would perhaps break binary compatibility, whereas an exception would likely not change much to existing users, and it would clarify that the websocket stream definitely ended, wheras a message subtype could convey the idea that the websocket will remain alive. An exception would also work in both directions: If the client closes the connection with a code Akka can throw the exception in the websocket stream and downstream consumer can handle it via recover; on the other hand the websocket handler flow could throw the exception to make Akka terminate the connection with the given code on the server side.

@jrudolph
Copy link
Contributor

I tend to favour a custom exception.

One problem might be that failures in a stream will discard buffers, so it cannot be easily guaranteed that all messages have been sent before the close. As long as we don't have good ideas whether that is the right thing to do, I'm still more interested in alternatives.

One other alternative could be to use specially crafted messages for this purpose. E.g. we could hide something in a streamed message by crafting a special kind of Source that could only be interpreted with internal knowledge (e.g. a Source.empty with an internal attribute). When materialized the source would be empty. An empty message could still confuse existing code, so receiving close codes from the peer should be opt-in. Sending such a close message would be supported in any case using a new message constructor that would craft such a close message from code and reason.

It's a bit hacky but maybe the best solution given the constraints?

@swsnr
Copy link

swsnr commented Mar 29, 2021

One problem might be that failures in a stream will discard buffers, so it cannot be easily guaranteed that all messages have been sent before the close.

If the client closes the connection do server-side buffers still matter?

If not we could use a hybrid approach: Throw an exception for client-side closes, and use a custom message for server-side closing. This would change the Flow type to e.g. Flow[ws.Message, ws.ServerMessage] where ServerMessage derives from Message and adds a CloseMessage.

The exception wouldn't break anything I believe; attempting to talk to a closed socket likely already throwns some other exception at some point.

Another outgoing message variant is perhaps also fine; I don't remember the variance of Flow, but even if it's invariant the changed type would only manifest when passing the flow to handleWebSocketMessages, and there are likely not many places this method is called.

@jrudolph
Copy link
Contributor

If the client closes the connection do server-side buffers still matter?

No, it's about buffers (implementation or user) in the stream that sends the close after the component that throws the failure.

E.g. if this user wants to close with code:

Flow[Message]
  .map { peerCmd =>
     if (cond) throw CloseNowException(code, reason)
  }
  .buffer(...) // or filter or mapAsync, etc.

In that case, anything behind the component that closes the connection might discard buffered data. Of course, you could document that for the user and require the user to make sure to buffer nothing. Then we would need to check our internals as well that we avoid losing data if possible. Still it makes it somewhat awkward if you want to do a (non-abort) regular close with a custom code.

I just looked again in the RFC as well, and it's not clear to me if there's a clear difference between "regular close with code" and "connection abort with code" and whether we would need to support both.

@swsnr
Copy link

swsnr commented Mar 29, 2021

@jrudolph I understand, but if we use a regular message for a server-side close buffering wouldn't be an issue, would it? The close message would be buffered just like a normal text or binary message after all.

So if we used an exception for a client side close, and a regular message for a server side close, as suggested in my previous, we'd be fine, wouldn't we?

The server could then do a regular close by just sending the close message regularly through the flow, including any buffering, or a abort which discards any buffers, by throwing a custom exception and then ultimately recovering with a Close message.

Client side closes would still always discard any buffers in the stream; but if the client's gone any buffers in the flow are lost anyway, since there's none left send their contents to.

@jrudolph
Copy link
Contributor

Not sure I get the distinction between server and client here. Both could be implemented with akka-http and on both sides you might be interested in only closing the connection after all buffered messages have been sent.

@swsnr
Copy link

swsnr commented Mar 31, 2021

I am sorry for the confusion; I was speaking from the perspective of an Akka HTTP server talking to another client because that's our use case. I'll try to rephrase what I said in terms of sources and sinks representing the websocket stream:

If the remote end connected to a Source of websocket messages closes with a code I think Akka HTTP could safely throw a RemoteEndClosedException; buffers of outgoing messages can no longer be transmitted anyway so buffers would also get lost currently, regardless of what Akka HTTP currently does in this case.

To let Akka HTTP close the websocket with a code on its own it could send a new CloseMessage to the outgoing Sink; if the sink buffers this message would be buffered just like all others before. This would change the type of a Sink for outgoing messages to Sink[OutgoingMessage]; but if OutgoingMessage was a sub-trait of ws.Message I think this would be as compatible as it could get.

Does this illustrate what I'm thinking about?

As said I'm would make a pull request in that direction, but since it's not a trivial thing I'd like to be sure of the direction to go into.

@rahmanash-dbz
Copy link

Any updates on this? I am stuck at an use case where connection should be closed after some time,based on time, token etc.
Need to close this with an application specific error message to handle things in client.

@mmacfadden
Copy link

To comment on the above suggestion to use an exception, I am not sure if that is the best approach. Closing of the web socket does not indicate an exceptional case. Closing is a normal part of the lifecycle. Once could argue that a close code other than 1000 could represent an exceptional case. In this way, you could argue that a normal close frame of 1000 would not generate the exception and any other close code would. However, even with the 1000 code case, there is a human readable reason that is part of the protocol that we could like access too.

In our use case for example, (on the server side) we would like to be able to distinguish code 1000 from code 1001. Neither of these are exceptional or error cases. The former indicates that the web socket was normally closed, the other indicates that the page is being navigated away from. In addition codes 4000-4999 are available for applications to use. These codes do not indicate that there has been an error of any kind, so handling them via an exception / recover would give the wrong connotation in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Identifies issues that the core team will likely not have time to work on t:core Issues related to the akka-http-core module t:websocket
Projects
None yet
Development

No branches or pull requests

8 participants