rabbitmq / rabbitmq-web-stomp

Provides support for STOMP over WebSockets
Other
89 stars 26 forks source link

Stop on errors #122

Closed lukebakken closed 4 years ago

lukebakken commented 4 years ago

Rather than crashing with case_clause, errors returned in the data handlers should result in the orderly shutdown of the STOMP connection.

Fixes #121

@essen I'm curious, I noticed that returning {stop, State} is a deprecated call result:

https://github.com/ninenines/cowboy/blob/2.6.1/src/cowboy_websocket.erl#L43-L47

What will eventually be the correct way to return stop?

lukebakken commented 4 years ago

@essen thanks for the explanation. The documentation and type specs (call_result) make more sense now as well.

lukebakken commented 4 years ago

@michaelklishin here is what is returned to the client in the error case now (from Wireshark):

WebSocket
    1... .... = Fin: True
    .000 .... = Reserved: 0x0
    .0.. .... = Per-Message Compressed: False
    .... 1000 = Opcode: Connection Close (8)
    1... .... = Mask: True
    .001 0101 = Payload length: 21
    Masking-Key: 3d69f6c0
    Masked payload
    Payload
        Close
            Status code: Invalid frame payload data (1007)
            Reason: {bad_escape,"\\\n"}
lukebakken commented 4 years ago

Backported to v3.8.x

michaelklishin commented 4 years ago

Thank you, @lukebakken.