Skip to content
This repository was archived by the owner on May 5, 2023. It is now read-only.

Uncaught TypeError: Cannot read property 'socket' of undefined #18

Merged
merged 2 commits into from
Apr 13, 2018

Conversation

talmobi
Copy link
Contributor

@talmobi talmobi commented Apr 3, 2018

Halåj, this is a patch for a bug when a connection fails we are trying to access a property on the undefined result object instead of first handling the actual connection error.

Opened an issue for this also in case others have same problem: #19

fix for Uncaught TypeError: Cannot read property 'socket' of undefined

bugfix: Cannot read property 'socket' of undefined

error: Uncaught TypeError: Cannot read property 'socket' of undefined

description:
handle connection error first before attempting to get result.socket
result will be undefined if there is a connection error, and instead
of handling the connection error, we will crash on trying to reference
socket from an 'undefined' result object

error: Uncaught TypeError: Cannot read property 'socket' of undefined

description:
handle connection error first before attempting to get result.socket
result will be undefined if there is a connection error, and instead
of handling the connection error, we will crash on trying to reference
socket from an 'undefined' result object
@talmobi
Copy link
Contributor Author

talmobi commented Apr 12, 2018

@TooTallNate I know you're busy, this is a very easy merge though~ thanks~

@knoxcard
Copy link
Contributor

@JoshGlazebrook

@JoshGlazebrook
Copy link
Contributor

JoshGlazebrook commented Apr 13, 2018

@talmobi This line just needs to be moved under the check for err. No need for a try/catch, result.socket will always be defined if err is null.

Old:

var socket = result.socket
if (err) return fn(err);

New:

if (err) return fn(err);
var socket = result.socket

need for a try/catch, result.socket will always be defined if err is
null.

ref: TooTallNate#18 (comment)
@TooTallNate TooTallNate merged commit 8d12d8f into TooTallNate:master Apr 13, 2018
@TooTallNate
Copy link
Owner

Thank you, published as v4.0.1.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants