Page MenuHomePhabricator

[lib] limit report size to 200MB
ClosedPublic

Authored by kamil on May 31 2023, 10:37 AM.
Tags
None
Referenced Files
F1699037: D8040.diff
Sat, May 4, 4:40 AM
F1699012: D8040.diff
Sat, May 4, 4:12 AM
Unknown Object (File)
Wed, Apr 17, 8:54 PM
Unknown Object (File)
Wed, Apr 17, 8:54 PM
Unknown Object (File)
Wed, Apr 17, 8:54 PM
Unknown Object (File)
Wed, Apr 17, 8:54 PM
Unknown Object (File)
Wed, Apr 17, 8:52 PM
Unknown Object (File)
Mar 3 2024, 9:43 PM
Subscribers

Details

Summary

Bigger reports will crash app anyway so avoid adding them to store.

I checked this with sizeof(std::string::value_type) in both iOS and Android, and (with no surprises) one char size is 1 byte. So assuming we can expect size to be approx string length without std::string overhead which was ~90000B with larger reports.

Test Plan

Generate large reports and verify that is not added

Diff Detail

Repository
rCOMM Comm
Branch
report-size
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
lib/utils/report-utils.js
20 ↗(On Diff #27315)

this can throw String length exceeds limit

kamil published this revision for review.Jun 1 2023, 1:26 AM
tomek added inline comments.
lib/utils/report-utils.js
17 ↗(On Diff #27315)

How about avoiding scientific notation?

20 ↗(On Diff #27315)

Is there a way to avoid allocating this potentially huge string? Wondering if there's a library, or other solution, that traverses object structure and measures it without converting to string.

This revision is now accepted and ready to land.Jun 5 2023, 4:04 AM

avoid scientific notation

lib/utils/report-utils.js
20 ↗(On Diff #27315)

We can just implement a simple recursive function that:

  • for string returns length
  • for number calls toString() and returns length
  • for array calls itself for each array element
  • for object calls itself for each value and adds each key length

and as a result, will return approximated string length - but I am skeptical as this recursive solution will probably be really slow. If you think it's better I can replace this later as a follow-up as this is relatively easy.

This revision was automatically updated to reflect the committed changes.