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

fallback for empty cursor.collection #193

Closed
wants to merge 1 commit into from
Closed

fallback for empty cursor.collection #193

wants to merge 1 commit into from

Conversation

ffflabs
Copy link

@ffflabs ffflabs commented Jan 28, 2015

Since I upgraded to node's MongoDB connector v2 (specifically, my package.json says "mongodb": "^2.0.13") I started getting a fatal error

TypeError: Cannot read property 'collectionName' of undefined
    at mongoCursorOperationProxy (/var/www/apppath/node_modules/newrelic/lib/instrumentation/mongodb.js:125:41)

The errors line is

var collection = cursor.collection.collectionName

and it seems that cursor.collection is empty now. BUT, I saw that cursor.ns has the form databaseName.collectionName, so I wrote a fallback to use that property instead if needed.

@seriousben
Copy link

Having the same problem with mongodb==2.0.15 happening on 'cursor.toArray' for me with the same trace.

@hayes
Copy link
Contributor

hayes commented Feb 5, 2015

Thanks for this PR. Unfortunately supporting the mondgodb >= 2.0 is a bit more complicated than this simple change. I am working on a full rewrite of our internal apis including the whole mongo instrumentation. We are hoping to release this very soon, and it should resolve the issues you are seeing in newer versions of the mongo driver. This is still a couple of weeks from release, but we are actively working on it.

best,
Michael

@seriousben
Copy link

Thanks a lot for your feedback
On Feb 5, 2015 4:54 PM, "Michael Hayes" notifications@github.com wrote:

Thanks for this PR. Unfortunately supporting the mondgodb >= 2.0 is a bit
more complicated than this simple change. I am working on a full rewrite of
our internal apis including the whole mongo instrumentation. We are hoping
to release this very soon, and it should resolve the issues you are seeing
in newer versions of the mongo driver. This is still a couple of weeks from
release, but we are actively working on it.

best,
Michael

Reply to this email directly or view it on GitHub
#193 (comment)
.

@seriousben
Copy link

Isn't there some way to just not break our apps even without supporting mongodb>=2.0?

@hayes
Copy link
Contributor

hayes commented Feb 11, 2015

We are still not quite ready to officially release the version of the node agent that has the fix for mongo 2.0 in it, but if you are interested in getting access to a beta version, please shoot me an email at mhayes@newrelic.com

@seriousben
Copy link

Is new release including fix for this?

@hayes
Copy link
Contributor

hayes commented Feb 26, 2015

Yep, should be fixed. Please let me know if you run into any additional
issues
On Feb 26, 2015 8:28 AM, "Benjamin Boudreau" notifications@github.com
wrote:

Is new release including fix for this?


Reply to this email directly or view it on GitHub
#193 (comment)
.

@hayes hayes closed this Feb 26, 2015
@seriousben
Copy link

Works great thanks a lot.
Maybe a small issue, I am seeing 3 different database for mongo while I am only talking to a single one. Might be related to connection pooling or something like that?

@hayes
Copy link
Contributor

hayes commented Feb 26, 2015

that sounds odd. If you send me a link (mhayes@newrelic.com) I'll be happy to take a look

bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 19, 2024
…c34296f32e8ad79db7

[Snyk] Security upgrade newrelic from 10.3.1 to 10.3.2
bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 23, 2024
…c34296f32e8ad79db7

[Snyk] Security upgrade newrelic from 10.3.1 to 10.3.2
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

3 participants