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.

Sign up for GitHub

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

Jump to bottom

crash in parsers.getContextFromRequest when req.socket is null #2479

Closed
trentm opened this issue Nov 30, 2021 · 0 comments · Fixed by #2480
Closed

crash in parsers.getContextFromRequest when req.socket is null #2479

trentm opened this issue Nov 30, 2021 · 0 comments · Fixed by #2480
Assignees
@trentm
Labels
agent-nodejs Make available for APM Agents project planning. bug

Comments

@trentm
Copy link
Member

trentm commented Nov 30, 2021

From nodejs/undici#1115 @sveisvei showed a stacktrace of a crash in [email protected]:

"type": "TypeError",
"message": "Cannot read properties of null (reading 'remoteAddress')",
"stack":
    TypeError: Cannot read properties of null (reading 'remoteAddress')
        at Object.getContextFromRequest (/node_modules/.pnpm/[email protected]/node_modules/elastic-apm-node/lib/parsers.js:28:34)
        at Transaction.toJSON (/node_modules/.pnpm/[email protected]/node_modules/elastic-apm-node/lib/instrumentation/transaction.js:189:41)
        at Transaction._encode (/node_modules/.pnpm/[email protected]/node_modules/elastic-apm-node/lib/instrumentation/transaction.js:218:15)
        at Instrumentation.addEndedTransaction (/node_modules/.pnpm/[email protected]/node_modules/elastic-apm-node/lib/instrumentation/index.js:284:63)
        at Transaction.end (/node_modules/.pnpm/[email protected]/node_modules/elastic-apm-node/lib/instrumentation/transaction.js:268:32)
        at ServerResponse.<anonymous> (/node_modules/.pnpm/[email protected]/node_modules/elastic-apm-node/lib/instrumentation/http-shared.js:41:36)
        at ServerResponse.f (/node_modules/.pnpm/[email protected]/node_modules/once/once.js:25:25)
        at ServerResponse.onfinish (/node_modules/.pnpm/[email protected]/node_modules/end-of-stream/index.js:31:27)
        at /node_modules/.pnpm/[email protected]/node_modules/elastic-apm-node/lib/instrumentation/run-context/AbstractRunContextManager.js:76:49
        at AsyncHooksRunContextManager.with (/node_modules/.pnpm/[email protected]/node_modules/elastic-apm-node/lib/instrumentation/run-context/BasicRunContextManager.js:49:17)
        at ServerResponse.wrapper (/node_modules/.pnpm/[email protected]/node_modules/elastic-apm-node/lib/instrumentation/run-context/AbstractRunContextManager.js:76:23)
        at ServerResponse.emit (node:events:402:35)
        at ServerResponse.emit (node:domain:475:12)
        at onFinish (node:_http_outgoing:830:10)
        at afterWrite (node:internal/streams/writable:497:5)
        at afterWriteTick (node:internal/streams/writable:484:10)
        at processTicksAndRejections (node:internal/process/task_queues:82:21)

This is parsers.getContextFromRequest not guarding against req.socket being null.

repro

I haven't been able to reproduce this yet. I'm missing some detail on how an undici request is somehow involved with the transaction.req that is being serialized.

@sveisvei Are you able to provide a bit more detail on how you reproduce this? I will definitely fix this repo to be more defensive on using req.socket, but I would love to have a repro.
@trentm trentm self-assigned this Nov 30, 2021
@trentm trentm added the bug label Nov 30, 2021
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Nov 30, 2021
@trentm trentm mentioned this issue Nov 30, 2021
Closed
trentm added a commit that referenced this issue Nov 30, 2021
@trentm
fix: crash in Transaction serialization if req.socket is null
82efcc4 
Fixes: #2479
@trentm trentm mentioned this issue Nov 30, 2021
Merged
3 tasks
trentm added a commit that referenced this issue Dec 2, 2021
@trentm
fix: crash in Transaction serialization if req.socket is null (#2480)
3928f8d 
Fixes: #2479
@ddolcimascolo ddolcimascolo mentioned this issue Feb 19, 2025
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@trentm trentm

Labels
agent-nodejs Make available for APM Agents project planning. bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: crash in Transaction serialization if req.socket is null elastic/apm-agent-nodejs
1 participant
@trentm