[lib] limit report size to 200MB
ClosedPublic
Actions

Authored by kamil on May 31 2023, 10:37 AM.

Details

Reviewers

michal
tomek

Commits

rCOMMdb576808a62e: [lib] limit report size to 200MB

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kamil created this revision.May 31 2023, 10:37 AM

kamil held this revision as a draft.

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptMay 31 2023, 10:37 AM

kamil added inline comments.May 31 2023, 10:38 AM

lib/utils/report-utils.js
20 ↗	(On Diff #27315)	this can throw `String length exceeds limit`

Harbormaster completed remote builds in B19930: Diff 27315.May 31 2023, 10:58 AM

kamil published this revision for review.Jun 1 2023, 1:26 AM

kamil added a reviewer: tomek.Jun 5 2023, 3:01 AM

tomek accepted this revision.Jun 5 2023, 4:04 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.