fixed "cannot read property aborted of null bug"

[helloIAmPau]

Hi all,

when I try to connect to an invalid host (one who does not listen for WebSocket connections) I get the following exception I can't handle in any way:

Uncaught TypeError: Cannot read property 'aborted' of null
    at ClientRequest._req.on (/develop/node-statwolf/packages/statwolf-debugger-client/node_modules/ws/lib/WebSocket.js:646)
    at emitOne (events.js:96)
    at ClientRequest.emit (events.js:188)
    at Socket.socketCloseListener (_http_client.js:285)
    at emitOne (events.js:101)
    at Socket.emit (events.js:188)
    at TCP._handle.close [as _onclose] (net.js:501)

It looks to be an invalid access to this._req variable.

In this merge-request, I just added a check that should fix the issue.
I hope it will be helpful.

[3rd-Eden]

Is there any way you could add a test for this?

[helloIAmPau]

sure, sorry about that.

[lpinca]

Can you please show us a small piece of code to reproduce the issue?
This can only happen if the 'error' event is emitted more than once or if the 'error' event is emitted after the 'upgrade' event and this should never happen.

[helloIAmPau]

It looks that the issue occurs when I try to connect to a host behind a firewall. Maybe is some kind of race condition connected to the HTTP timeout. Tell me if you can reproduce this so that I proceed to write some test for the fix.

[lpinca]

@helloIAmPau I cannot reproduce the issue. Can you investigate a bit further and see what events are emitted and in what order?

Thank you.

[lpinca]

Ping @helloIAmPau.

[lpinca]

Closing as I think the issue happens due to a missing 'error' listener on the WebSocket instance and the use of the 'uncaughtException' event as discussed in #1229.

Please reopen if this is not the case.

[helloIAmPau]

Thank you, I'll check the issue.

On Sat, Nov 4, 2017, 12:54 PM Luigi Pinca ***@***.***> wrote: Closed #1201 <#1201>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1201 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-h2LBCl-ksOv390qGVEIm8b9M0ubvgks5szFBpgaJpZM4PRD1H> .

-- P.

[connor4312]

Hey @lpinca I just ran into this today on 3.3.2. The remote server gave a ECONNRESET at a bad time which resulted in this error being thrown (and then swallowing the error event so we didn't reconnect!)

Uncaught error: Error: read ECONNRESET
    at _errnoException (util.js:1024:11)
    at TLSWrap.onread (net.js:615:25)
Uncaught error: TypeError: Cannot read property 'aborted' of null
    at ClientRequest._req.on (/Users/copeet/Github/node-xbl/node_modules/ws/lib/WebSocket.js:634:19)
    at emitOne (events.js:116:13)
    at ClientRequest.emit (events.js:211:7)
    at TLSSocket.socketCloseListener (_http_client.js:363:9)
    at emitOne (events.js:121:20)
    at TLSSocket.emit (events.js:211:7)
    at _handle.close (net.js:554:12)
    at TCP.done [as _onclose] (_tls_wrap.js:356:7)

[lpinca]

@connor4312 that "Uncaught error" looks suspicious, are you using 'uncaughtException'? If so, remove it, as it makes the 'error' event on the http.ClientRequest object to be emitted twice.

[connor4312]

The "uncaught error" is our own message! We use uncaughtException in order to log telemetry about the error to Sentry. It seems odd that that would be incompatible with this library--would that be considered a bug in ws?

[lpinca]

@connor4312 do you have an 'error' event listener on the WebSocket instance?

If so and the listener isn't called then the issue is Node.js as we are forwarding it from http.ClientRequest.

If not and you use 'uncaughException' you are doing it wrong as you can't resume operations. In this case it makes the error event to be emitted twice.

[lpinca]

See also #1229 (comment) if you didn't already.