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

Support for other frameworks that not expose raw request at req.raw.req #78

Open
nicomf1982 opened this issue Apr 15, 2021 · 3 comments
Assignees
Labels
agent-nodejs Make available for APM Agents project planning. bug Something isn't working

Comments

@nicomf1982
Copy link

Hello guys:

After jump from previous version (0.1...0.2) to 1.0 i found that my REQ object isn't logged anymore...
Digger into the code, i found that the expected shape of the req object (IncomingMessage) is now: req -> raw-> req , and other frameworks, like fastify, expose that at req -> raw directly.

I have a fix to support this (and possibly others (?) frameworks), let me know to send PR.

thx

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Apr 15, 2021
@nicomf1982 nicomf1982 changed the title Support for other frameworks that not expose raw req at req.raw.req Support for other frameworks that not expose raw request at req.raw.req Apr 15, 2021
@nishkarsh-beamery
Copy link

Is there going to be any action taken on this issue?

@trentm
Copy link
Member

trentm commented Oct 30, 2023

@nicomf1982 Thanks for the issue. My apologies for never having replied. I understand if you have moved on by now.

If this is still relevant to you, I would like to discuss it now and look into a fix.

I believe the code block you are referring to is this:

  if (req.raw && req.raw.req && req.raw.req.httpVersion) {
    // This looks like a hapi request object (https://hapi.dev/api/#request),
    // use the raw Node.js http.IncomingMessage that it references.
    // TODO: Use hapi's already parsed `req.url` for speed.
    req = req.raw.req
  }

This was added to support @hapi/hapi.

My expectation is that this would not have broken usage for Fastify, which has req.raw (https://fastify.dev/docs/latest/Reference/Request/) ... unless I am mistaken and req.raw.req is actually truthy. The repo does not have a specific test case for Fastify, however. I will look into adding one.

@trentm trentm self-assigned this Oct 30, 2023
@trentm
Copy link
Member

trentm commented Oct 31, 2023

I can reproduce the issue. The change that broke this is #69. That "defensive" handling:

  // Use duck-typing to check this is a `http.IncomingMessage`-y object.
  if (!('httpVersion' in req && 'headers' in req && 'method' in req)) {
    return false
  }

results in formatHttpRequest() returning early. I will get on a fix for this now.

@trentm trentm added the bug Something isn't working label Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning. bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants