-
Notifications
You must be signed in to change notification settings - Fork 318
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
NodeJS 20: dd-trace reports HTTP requests with duration above 5 seconds as errors #3515
Comments
Glancing through the code, does dispatching a finish and error events here make sense on a According to https://nodejs.org/api/http.html#event-timeout:
This doesn't mean that the HTTP request itself is timing out. For example, if a request takes a long time (> 5 s), the timeout event will be emitted depending on the Should we be listening to the |
Here's a quick app that makes a request to a service that takes 6 seconds to respone: require('dd-trace/init');
fetch('https://postman-echo.com/delay/6').then((d) => {
console.error('success:', d.status);
}).catch((err) => {
console.error('error:', err);
}); This app can be executed like so: DD_TRACE_DEBUG=1 DD_TRACE_BEAUTIFUL_LOGS=1 node app.js In the output I see the following:
These results suggest everything worked as expected. I also tried modifying the app to include a timeout: require('dd-trace/init');
fetchWithTimeout('https://postman-echo.com/delay/6', {timeout: 2000}).then((d) => {
console.error('success:', d.status);
}).catch((err) => {
console.error('error:', err);
});
async function fetchWithTimeout(resource, options = {}) {
const { timeout = 8000 } = options;
const controller = new AbortController();
const id = setTimeout(() => controller.abort(), timeout);
const response = await fetch(resource, {
...options,
signal: controller.signal
});
clearTimeout(id);
return response;
} The output doesn't seem to show any repeated events:
When I modify the apps to use your more complex Can you modify my sample app to reproduce the 5 second error that you're encountering? I didn't see any 5 second timeouts in dd-trace itself. |
@tlhunter Thanks for taking a look. We have the same setup except that you hard-coded the timeout to be 2000 ms there. Try to set the timeout to be greater than 5000 ms (leaving the timeout as 8000 ms is fine). Also, try to use this for the fetch package:
from: https://www.npmjs.com/package/cross-fetch
|
@sabrenner @tlhunter Thank you for fixing this issue. You both rock! |
When we upgraded to Node.js 20, dd-trace doesn't report HTTP requests with durations above 5 seconds correctly. An example trace in Datadog:
I added an additional log to see if the request times out. It doesn't seem to be the case. The request took a long time ( > 5 seconds), but succeeded.
I verified that this doesn't happen when the requests take less than 5 seconds.
Expected behaviour
dd-trace should accurately report requests that take above 5 seconds with their duration and status code.
Actual behaviour
dd-trace report requests that take above 5 seconds as errors, and report their durations as ~5 seconds.
Steps to reproduce
Environment
The text was updated successfully, but these errors were encountered: