Skip to content

[core] Ensure Explicit Timeouts from Underlying Request Socket are Recorded as Errors When Using Node 20#3853

Merged
sabrenner merged 2 commits intomasterfrom
sabrenner/http-socket-default-timeout
Dec 8, 2023
Merged

[core] Ensure Explicit Timeouts from Underlying Request Socket are Recorded as Errors When Using Node 20#3853
sabrenner merged 2 commits intomasterfrom
sabrenner/http-socket-default-timeout

Conversation

@sabrenner
Copy link
Copy Markdown
Collaborator

What does this PR do?

Makes sure intentional setTimeout calls on the underlying socket for a request are captured for recording errors on timeouts when using Node 20. See the linked PR below, but because Node 20 defaults to 5s timeouts on the global agent, we want to ignore those timeouts as errors, but record intentional ones as errors.

Motivation

Addresses concern after merging #3841

Security

Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 8, 2023

Overall package size

Self size: 5.66 MB
Deduped: 61.69 MB
No deduping: 62.45 MB

Dependency sizes

name version self size total size
@datadog/native-iast-taint-tracking 1.6.4 16.43 MB 16.44 MB
@datadog/native-appsec 5.0.0 15.16 MB 15.17 MB
@datadog/pprof 4.1.0 9.36 MB 10.21 MB
protobufjs 7.2.4 2.74 MB 6.52 MB
@datadog/native-iast-rewriter 2.2.1 2.27 MB 2.36 MB
@opentelemetry/core 1.14.0 872.87 kB 1.47 MB
@datadog/native-metrics 2.0.0 898.77 kB 1.3 MB
@opentelemetry/api 1.4.1 780.32 kB 780.32 kB
import-in-the-middle 1.4.2 41.4 kB 704.79 kB
pprof-format 2.0.7 588.12 kB 588.12 kB
msgpack-lite 0.1.26 201.16 kB 281.59 kB
opentracing 0.14.7 194.81 kB 194.81 kB
semver 7.5.4 93.4 kB 123.8 kB
@datadog/sketches-js 2.1.0 109.9 kB 109.9 kB
lodash.sortby 4.7.0 75.76 kB 75.76 kB
lru-cache 7.14.0 74.95 kB 74.95 kB
ipaddr.js 2.1.0 60.23 kB 60.23 kB
ignore 5.2.4 51.22 kB 51.22 kB
int64-buffer 0.1.10 49.18 kB 49.18 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
lodash.uniq 4.5.0 25.01 kB 25.01 kB
limiter 1.1.5 23.17 kB 23.17 kB
dc-polyfill 0.1.2 22.77 kB 22.77 kB
retry 0.13.1 18.85 kB 18.85 kB
lodash.kebabcase 4.1.1 17.75 kB 17.75 kB
node-abort-controller 3.1.1 16.89 kB 16.89 kB
lodash.pick 4.4.0 16.33 kB 16.33 kB
jest-docblock 29.7.0 8.99 kB 12.76 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 kB
path-to-regexp 0.1.7 6.78 kB 6.78 kB
koalas 1.0.2 6.47 kB 6.47 kB
methods 1.1.2 5.29 kB 5.29 kB
module-details-from-path 1.0.3 4.47 kB 4.47 kB

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9c71b30) 84.95% compared to head (aa383e6) 87.61%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3853      +/-   ##
==========================================
+ Coverage   84.95%   87.61%   +2.65%     
==========================================
  Files         233      227       -6     
  Lines        9637     9017     -620     
  Branches       33       33              
==========================================
- Hits         8187     7900     -287     
+ Misses       1450     1117     -333     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sabrenner sabrenner marked this pull request as ready for review December 8, 2023 16:27
@sabrenner sabrenner requested review from a team as code owners December 8, 2023 16:27
@sabrenner sabrenner requested a review from jbertran December 8, 2023 16:27
Copy link
Copy Markdown
Member

@rochdev rochdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be safer to use shimmer to do the patching, but this method is extremely unlikely to change to it should be fine.

@sabrenner sabrenner merged commit 59c8ea4 into master Dec 8, 2023
@sabrenner sabrenner deleted the sabrenner/http-socket-default-timeout branch December 8, 2023 17:38
khanayan123 pushed a commit that referenced this pull request Dec 12, 2023
…corded as Errors When Using Node 20 (#3853)

* req.socket timeouts recorded as errors node 20

* check for process.send
khanayan123 pushed a commit that referenced this pull request Dec 13, 2023
…corded as Errors When Using Node 20 (#3853)

* req.socket timeouts recorded as errors node 20

* check for process.send
This was referenced Dec 13, 2023
khanayan123 pushed a commit that referenced this pull request Dec 14, 2023
…corded as Errors When Using Node 20 (#3853)

* req.socket timeouts recorded as errors node 20

* check for process.send
khanayan123 pushed a commit that referenced this pull request Dec 14, 2023
…corded as Errors When Using Node 20 (#3853)

* req.socket timeouts recorded as errors node 20

* check for process.send
Qard pushed a commit that referenced this pull request Dec 20, 2023
…t are Recorded as Errors When Using Node 20 (#3853)"

This reverts commit 59c8ea4.
Qard pushed a commit that referenced this pull request Dec 20, 2023
…t are Recorded as Errors When Using Node 20 (#3853)" (#3896)

This reverts commit 59c8ea4.
hoolioh pushed a commit that referenced this pull request Dec 21, 2023
…t are Recorded as Errors When Using Node 20 (#3853)" (#3896)

This reverts commit 59c8ea4.
@hoolioh hoolioh mentioned this pull request Dec 21, 2023
hoolioh pushed a commit that referenced this pull request Dec 21, 2023
…t are Recorded as Errors When Using Node 20 (#3853)" (#3896)

This reverts commit 59c8ea4.
@hoolioh hoolioh mentioned this pull request Dec 21, 2023
CarlesDD pushed a commit that referenced this pull request Dec 21, 2023
…t are Recorded as Errors When Using Node 20 (#3853)" (#3896)

This reverts commit 59c8ea4.
CarlesDD pushed a commit that referenced this pull request Dec 21, 2023
…t are Recorded as Errors When Using Node 20 (#3853)" (#3896)

This reverts commit 59c8ea4.
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
…corded as Errors When Using Node 20 (#3853)

* req.socket timeouts recorded as errors node 20

* check for process.send
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
…t are Recorded as Errors When Using Node 20 (#3853)" (#3896)

This reverts commit 59c8ea4.
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
…corded as Errors When Using Node 20 (#3853)

* req.socket timeouts recorded as errors node 20

* check for process.send
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
…t are Recorded as Errors When Using Node 20 (#3853)" (#3896)

This reverts commit 59c8ea4.
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
…corded as Errors When Using Node 20 (#3853)

* req.socket timeouts recorded as errors node 20

* check for process.send
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
…t are Recorded as Errors When Using Node 20 (#3853)" (#3896)

This reverts commit 59c8ea4.
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
…corded as Errors When Using Node 20 (#3853)

* req.socket timeouts recorded as errors node 20

* check for process.send
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
…t are Recorded as Errors When Using Node 20 (#3853)" (#3896)

This reverts commit 59c8ea4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants