Skip to content

fix(web-common): add limit for timeout#6601

Merged
pichlermarc merged 6 commits intoopen-telemetry:mainfrom
maryliag:fix-timeout
Apr 23, 2026
Merged

fix(web-common): add limit for timeout#6601
pichlermarc merged 6 commits intoopen-telemetry:mainfrom
maryliag:fix-timeout

Conversation

@maryliag
Copy link
Copy Markdown
Contributor

_maxDuration * 1000 and _inactivityTimeout * 1000 can overflow if the config value is large (max safe setTimeout delay is 2147483647, ~24.8 days)
If elapsed time exceeds the max duration possible, setTimeout treats that as 1, firing the reset almost immediately

This PR adds a limit to those values.

@maryliag maryliag requested review from a team as code owners April 16, 2026 15:32
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.74%. Comparing base (60c08f1) to head (3c12eab).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6601   +/-   ##
=======================================
  Coverage   95.74%   95.74%           
=======================================
  Files         375      375           
  Lines       12818    12820    +2     
  Branches     3056     3056           
=======================================
+ Hits        12272    12274    +2     
  Misses        546      546           
Files with missing lines Coverage Δ
...rimental/packages/web-common/src/SessionManager.ts 88.73% <100.00%> (+0.32%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@overbalance overbalance left a comment

Choose a reason for hiding this comment

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

You may want to use a constant for the magic number now that there are two instances of 2147483647

Comment thread experimental/packages/web-common/src/SessionManager.ts Outdated
Comment thread experimental/packages/web-common/src/SessionManager.ts Outdated
Comment thread experimental/packages/web-common/src/SessionManager.ts Outdated
@pichlermarc pichlermarc added the browser Browser-specific additions or benefits label Apr 22, 2026
Copy link
Copy Markdown
Contributor

@overbalance overbalance left a comment

Choose a reason for hiding this comment

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

nit: add units to the end of the vars (if that is the convention in other files): _MS

@maryliag
Copy link
Copy Markdown
Contributor Author

done

@pichlermarc pichlermarc enabled auto-merge April 23, 2026 07:24
@pichlermarc pichlermarc added this pull request to the merge queue Apr 23, 2026
Merged via the queue into open-telemetry:main with commit 08c178e Apr 23, 2026
27 checks passed
@maryliag maryliag deleted the fix-timeout branch April 23, 2026 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

browser Browser-specific additions or benefits

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants