Rapsssito / react-native-tcp-socket

React Native TCP socket API for Android, iOS & macOS with SSL/TLS support.
MIT License
316 stars 81 forks source link

Add error callback to connect method #105

Closed VladLupashevskyi closed 3 years ago

Rapsssito commented 3 years ago

@VladLupashevskyi, thanks for the PR! Unfortunately it cannot be merged because it will break the cross-platform API with NodeJS. react-native-tcp-socket mimics Node's net API, so every interface must match net's.

In this case, take a look at the connect method. As it is stated in the docs, if an error occurs, the error event will be emitted. Adding a new parameter is not necessary.

VladLupashevskyi commented 3 years ago

@Rapsssito thanks you the quick reply.

I'm not familiar very much with NodeJS net API, but I see your point.

However unexpected issue still can happen. Take a look at this example:

const connect = createConnection(...);
// Delay 500ms
connect.on('error', console.error);

In this case the error can happen before handler is attached.

I assume in NodeJS net library one can instantiate the "connect" object and attach a handler BEFORE actually initializing connection, so it's always guaranteed that error handler will be called on error.

Taking in account cross-platform support we can do the following:

  1. Revert changes in connect function in TcpSocket.js.
  2. Introduce new function in index.js like:
function createConnectionOrError(options, success, error) {
    const tcpSocket = new Socket(getInstanceNumber(), nativeEventEmitter);
    tcpSocket.on('error', error);
    return tcpSocket.connect(options, success);
}

@Rapsssito What do you think about it?

Rapsssito commented 3 years ago

@VladLupashevskyi I have just pushed a new version. Now, you are able to instantiate a Socket without connecting, so you should be able to add the error listener before connecting. Check out the example code for more info.