DataDog / dd-trace-js

JavaScript APM Tracer
https://docs.datadoghq.com/tracing/
Other
646 stars 305 forks source link

Cannot read properties of undefined (reading 'span') #2915

Open MatthewMaclean opened 1 year ago

MatthewMaclean commented 1 year ago

Expected behaviour Tracer shouldn't crash the server on failure. Is there a

Actual behaviour The tracer crashed our server with the following error:

uncaughtException: Cannot read properties of undefined (reading 'span')
TypeError: Cannot read properties of undefined (reading 'span')
    at /opt/render/project/src/node_modules/dd-trace/packages/datadog-plugin-http2/src/client.js:75:38
    at Subscription._handler (/opt/render/project/src/node_modules/dd-trace/packages/dd-trace/src/plugins/plugin.js:14:9)
    at Channel.publish (node:diagnostics_channel:56:9)
    at /opt/render/project/src/node_modules/dd-trace/packages/datadog-instrumentations/src/http2/client.js:17:29
    at exports.AsyncResource.runInAsyncScope (node:async_hooks:202:9)
    at ClientHttp2Stream.emit (/opt/render/project/src/node_modules/dd-trace/packages/datadog-instrumentations/src/http2/client.js:14:23)
    at emit (node:internal/http2/core:328:3)
    at processTicksAndRejections (node:internal/process/task_queues:86:22)

I'm not familiar enough with the codebase, but it's weird that https://github.com/DataDog/dd-trace-js/blob/v3.14.1/packages/dd-trace/src/plugins/plugin.js#L13 would call into a function that expects the store to exist: https://github.com/DataDog/dd-trace-js/blob/v3.14.1/packages/datadog-plugin-http2/src/client.js#L74

Steps to reproduce Happened randomly on server startup, haven't seen it reproduce. There were no recent library upgrades.

Environment

ZebGirouard commented 1 year ago

+1, I'm seeing this error this week on some of our Jest CI runs

gratus-acuitymd commented 1 year ago

+1, encountered this as well - was intermittent and went undetected during development.

MatthewMaclean commented 11 months ago

My solution here is: tracer.use('http2', { enabled: false });

It's not ideal, but I prefer it over my application crashing due to tracing observability. The unfortunate part here is that the exception is uncatchable outside of the library, based on where they have to hook in. I can understand that solving the underlying issue might be difficult, but I think the library should at least include some nullability checks for preventative measures.