Skip to content

Conversation

@atomantic
Copy link
Contributor

TypeError: Object.keys called on non-object
    at Function.keys (native)
    at Object.parseParameters (/home/cloud-user/did_groups_api/node_modules/newrelic/lib/util/urltils.js:78:14)
    at TraceSegment.markAsWeb (/home/cloud-user/did_groups_api/node_modules/newrelic/lib/transaction/trace/segment.js:91:32)
    at ServerResponse.instrumentedFinish (/home/cloud-user/did_groups_api/node_modules/newrelic/lib/instrumentation/core/http.js:127:15)
    at ServerResponse.g (events.js:199:16)
    at ServerResponse.<anonymous> (/home/cloud-user/did_groups_api/node_modules/newrelic/node_modules/continuation-local-storage/context.js:74:17)
    at ServerResponse.emit (events.js:129:20)
    at ServerResponse.emitted [as emit] (/home/cloud-user/did_groups_api/node_modules/newrelic/node_modules/continuation-local-storage/node_modules/emitter-listener/listener.js:122:21)
    at finish (_http_outgoing.js:517:10)
    at /home/cloud-user/did_groups_api/node_modules/newrelic/node_modules/continuation-local-storage/node_modules/async-listener/glue.js:188:31

At least in node 0.11.14, parsed.search is null, not an empty string when query is null:

parsed { protocol: null,
  hostname: null,
  slashes: null,
  hash: null,
  auth: null,
  search: null,
  host: null,
  query: null,
  port: null,
  pathname: '/favicon.ico',
  hostname: null,
  path: '/favicon.ico',
  hash: null,
  href: '/favicon.ico' }
  search: null,
  query: null,
  pathname: '/favicon.ico',
  path: '/favicon.ico',
  href: '/favicon.ico' }

@atomantic
Copy link
Contributor Author

I'd rather if (parsed.search) {, but not sure if you are explicitly checking for values as a code style or for a particular reason. Let me know if if (parsed.search) { is acceptable and I'll update the pull request.

@wraithan
Copy link
Contributor

@atomantic Hey, thanks a ton for this patch. If you don't mind squashing this down into a single commit (without the merge commit) I could see merging it for the release this week. I'll need to verify that it doesn't cause any weird issues on 0.8 or 0.10, but it looks pretty good to me.

@wraithan
Copy link
Contributor

I gave this patch a go on 0.11.14, this appears to fix things enough for the agent to start reporting on 0.11.14, but I'm unsure of any other problems that may arise. I'll be trying other versions soon to make sure nothing broke, but just wanted to verify your fix for the version you were targeting.

@atomantic
Copy link
Contributor Author

cleaned. Thanks for taking a look. Forced into using node 0.11.14 to get cluster support for PM2 :)

@wraithan
Copy link
Contributor

👍 from me on this. I'll make sure it gets into this week's release, I'm unclear on if it will make it into release notes as we don't officially support 0.11.x. I'll check in about that. Do you have a preferred way for us to attribute? We can use your name as it appears on github, or your github handle, or anything else you'd like.

@atomantic
Copy link
Contributor Author

Thanks. github name is fine :)

@wraithan
Copy link
Contributor

@atomantic Your patch was merged out of band (and rebased). Thanks for the PR!

@wraithan wraithan closed this Oct 16, 2014
jsumners-nr pushed a commit to jsumners-nr/node-newrelic that referenced this pull request Apr 16, 2024
bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Jul 26, 2024
feat: Added a test suite for App Router.
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.

2 participants