Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update normalizeConnectArgs to latest master #244

Closed
wants to merge 1 commit into from

Conversation

LinusU
Copy link

@LinusU LinusU commented Mar 13, 2017

Fixes the following error with pg:

TypeError: "listener" argument must be a function
    at Socket.once (events.js:307:11)
    at Socket.connect (net.js:943:10)
    at wrappedConnect (/node_modules/newrelic/lib/instrumentation/core/net.js:53:31)
    at wrapped (/node_modules/newrelic/lib/transaction/tracer/index.js:183:28)
    at Tracer.addSegment (/node_modules/newrelic/lib/transaction/tracer/index.js:83:48)
    at Socket.connectWrapper [as connect] (/node_modules/newrelic/lib/instrumentation/core/net.js:43:27)
    at Connection.connect (/node_modules/pg/lib/connection.js:66:17)
    at Client.connect (/node_modules/pg/lib/client.js:56:9)
    at BoundPool.Pool._create (/node_modules/pg-pool/index.js:68:10)
    at Pool._createResource (/node_modules/generic-pool/lib/generic-pool.js:325:17)
    at Pool.dispense [as _dispense] (/node_modules/generic-pool/lib/generic-pool.js:313:12)
    at Pool.acquire (/node_modules/generic-pool/lib/generic-pool.js:388:8)
    at BoundPool.<anonymous> (/node_modules/pg-pool/index.js:83:15)
    at BoundPool.Pool._promiseNoCallback (/node_modules/pg-pool/index.js:47:7)
    at BoundPool.Pool.connect (/node_modules/pg-pool/index.js:81:15)
    at PG.connect (/node_modules/pg/lib/index.js:77:15)
    [...]

@NatalieWolfe
Copy link
Contributor

Hi Linus, thank you for the contribution. This is actually a bug in Node 7.7.2 (nodejs/node#11761). We recommend you stick with 7.7.1 until that fix is published.

@LinusU
Copy link
Author

LinusU commented Mar 14, 2017

Ahh, that's interesting, I was quite sure that it started working after I applied this patch 🤔 maybe I messed something else up... will check...

options.host = args[1]
}
}

var cb = args[args.length - 1]
return typeof cb === 'function' ? [options, cb] : [options]
return (typeof cb === 'function') ? [options, cb] : [options, null]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your patch bypasses the Node bug by passing null for the callback when no callback is provided instead of nothing. This just happens to be the one thing that Socket#connect code does check for: https://github.com/nodejs/node/blob/v7.7.2/lib/net.js#L942.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, yeah that explains it then 😄

@NatalieWolfe
Copy link
Contributor

7.7.3 came out yesterday (https://nodejs.org/en/blog/release/v7.7.3/). Would you mind seeing if that version of node works for you?

@LinusU
Copy link
Author

LinusU commented Mar 15, 2017

I'll check tomorrow 👌

@NatalieWolfe
Copy link
Contributor

Just checking in if you've had an opportunity to test the newer version of Node?

@LinusU
Copy link
Author

LinusU commented Mar 22, 2017

Sorry totally forgot about this, yes it seems to be working in 7.7.3, thanks for the help 👏

@LinusU LinusU closed this Mar 22, 2017
@NatalieWolfe
Copy link
Contributor

Awesome! Glad it worked out. :)

bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 19, 2024
bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants