facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
118.17k stars 24.21k forks source link

WebSocket onclose handler: event.code and event.reason are both undefined. #14617

Closed jondubois closed 7 years ago

jondubois commented 7 years ago

Description

This happened in RN 0.44.0. It doesn't happen in the simulator, only on the real iPhone device (iPhone 6).

When you lock the phone while the app is running and you have an open WebSocket, the onclose handler will eventually be triggered and an event object will be passed to the handler. This event object doesn't have an event.code or event.reason property defined (which is inconsistent with all major browsers which always pass back a status code) so there is no way to tell what kind of error/condition caused the connection to become closed.

I'm not sure if this is in breach of the WebSocket RFC but it's not ideal because it's inconsistent with major browsers (e.g. when closing the latop and it goes to sleep, I get a 1006 in Chrome).

See original issue: https://github.com/SocketCluster/socketcluster-client/issues/83

Reproduction Steps

  1. Open application
  2. Create a new WebSocket connection. E.g. var socket = new WebSocket(uri);
  3. Attach onclose handler. E.g. socket.onclose = function (event) { console.log('close:', event.code, event.reason); }
  4. Lock phone
  5. Wait a bit
  6. Unlock phone
  7. event.code should be logged as undefined.

Sample Code

var socket = new WebSocket(uri);
socket.onclose = function (event) {
  console.log('close:', event.code, event.reason);
}

Solution

According to the WebSocket RFC, if no status code is given as part of a close control frame, then the code is assumed to be 1005 (assuming that a close control frame is sent) or 1006 (assuming that the client was closed 'abnormally' - Which typically means terminated immediately without sending a close control frame over the wire). See https://tools.ietf.org/html/rfc6455#section-7.1.5

Additional Information

lll000111 commented 7 years ago

Related ticket (duplicate - merge?): https://github.com/facebook/react-native/issues/12678

jondubois commented 7 years ago

@lll000111 Yes seems like.