Skip to content

fix: DH-20255 Limit maximum depth to serialize redux log data#2544

Merged
ericlln merged 3 commits intodeephaven:mainfrom
ericlln:DH-20255-fix-log-export-again
Oct 7, 2025
Merged

fix: DH-20255 Limit maximum depth to serialize redux log data#2544
ericlln merged 3 commits intodeephaven:mainfrom
ericlln:DH-20255-fix-log-export-again

Conversation

@ericlln
Copy link
Copy Markdown
Contributor

@ericlln ericlln commented Oct 6, 2025

Follow up PR to #2531 which failed testing, with an error from JSON.stringify() of RangeError: Invalid string length.

These errors are caused by linked structures such as a Java LinkedHashMap that's included in the Redux data in various API related keys. We use the blacklist to filter out keys containing these structures, but these keys are not stable and will need to be constantly updated to prevent the log export from outright failing.

To preemptively deal with this, the maximum depth to serialize to can be limited in the safe-stable-stringify library to avoid linked structures from blowing up the log size. With no blacklist entries, the exported log size is only 1.2MB.

With an updated blacklist, exporting the logs with or without the limit both yield log files of almost the exact same size, meaning that no useful data has been lost by limiting the depth. However, I'm open to suggestions to the default depth to serialize to, or whether it should be optional to have a limit on the depth instead of mandating a limit (Infinity is not accepted by the library).

@ericlln ericlln self-assigned this Oct 6, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.96%. Comparing base (a19bc11) to head (3701393).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/log/src/LogExport.ts 57.14% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2544   +/-   ##
=======================================
  Coverage   44.95%   44.96%           
=======================================
  Files         765      765           
  Lines       42920    42924    +4     
  Branches    10799    10801    +2     
=======================================
+ Hits        19296    19299    +3     
- Misses      23611    23612    +1     
  Partials       13       13           
Flag Coverage Δ
unit 44.96% <57.14%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 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.

@ericlln ericlln marked this pull request as ready for review October 6, 2025 21:51
@ericlln ericlln requested a review from mofojed October 6, 2025 21:51
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Do you have examples of the keys that were problematic?

Comment thread packages/log/src/LogExport.ts Outdated
@ericlln ericlln requested a review from mofojed October 7, 2025 13:15
['layoutStorage'],
['storage'],

// Below are confirmed enterprise specific keys, and will be moved in DH-20410
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

draftManager is also an enterprise specific key.
Okay to have this here for now, but on Enterprise side we should just add to the list at https://github.com/deephaven-ent/iris/blob/rc/sanluis/web/client-ui/src/utils/exportEnterpriseLogs.ts#L18

@ericlln ericlln merged commit 9cd6593 into deephaven:main Oct 7, 2025
11 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants