feat(replay): Truncate network bodies to max size #7875
Conversation
size-limit report 📦
|
billyvg
left a comment
billyvg
left a comment
There was a problem hiding this comment.
Looks good to me. Would it be worth benchmarking fixJson against existing libraries (or even against something intensive in our sdk like compression) just to sanity check ourselves?
|
|
||
| /** | ||
| * Takes an incomplete JSON string, and returns a hopefully valid JSON string. | ||
| * Note that this _can_ fail, so you should check the return value is valid JSON. |
There was a problem hiding this comment.
What would cause it to fail? If it fails, what would be the result? Would it be better to throw in that case?
There was a problem hiding this comment.
Basically, we only know it failed once we tried to JSON.parse() it. It can fail in two cases:
- We incorrectly identified a string as JSON (e.g. think:
{ this is not JSON }). It will still not be JSON after our fixing, in this case 😅 - It was correct JSON, but something went wrong in the implementation. This should not happen, and I know of no case where this would happen, but it is reasonably complex that I wouldn't feel comfortable stating that it will never result in broken JSON.
My first implementation actually try-catched JSON.parse() here and threw an error, but the current implementation only has a single JSON.parse in networkUtils, where we try-catch this in a central place, also making it easier to generate the correct warnings for the _meta field.
|
|
||
| // Nested object/arrays | ||
| ['{"a":{"bb', '{"a":{"bb~~":"~~"}}'], | ||
| ['{"a":["bb",["cc","d', '{"a":["bb",["cc","d~~"]]}'], |
There was a problem hiding this comment.
Maybe a few test cases with an array of nested objects
There was a problem hiding this comment.
true, can add a few more here!
There was a problem hiding this comment.
I added some more tests, esp. with a large JSON body of nested stuff (found some issues while doing this, great!)
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
What a nice implementation! 🚀 You should really think about extracting fixJson to its own library!
| } catch { | ||
| return { | ||
| body, | ||
| warnings: ['INVALID_JSON'], |
There was a problem hiding this comment.
So it seems we can end up in catch either because we didn't hit the size limit and something went wrong with parsong, or because the size limit was hit, we truncated and repared and then something went wrong while parsing. To distinguish these cases, should we add a JSON_TRUNCATED (or perhaps a new warning) to warnings?
(I guess this might be helpful for debugging later on but I'm probably missing a lot of context on what we do with these warnings. Feel free to disregard if this isn't helpful).
There was a problem hiding this comment.
yeah, it's a good point, I wasn't sure. We can do INVALID_JSON & JSON_TRUNCATED together here, in that case, if that is helpful for the UI. @billyvg WDYT? we just need to make sure to handle this case properly in the UI then (when both of these are present)!
mydea
force-pushed
the
fn/replay-body-truncate
branch
from
6c34d3a to
e20f9ec
Compare
mydea
force-pushed
the
fn/replay-body-truncate
branch
from
1186380 to
3a18365
Compare
Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
mydea
force-pushed
the
fn/replay-body-truncate
branch
from
d681d6d to
57efea8
Compare
This PR updates the replay network body capture by truncating bodies to the max size of 150k characters.
The key part is, that we try to fix truncated JSON, if possible, so it remains a valid JSON object even after truncating.
For this, the key design goals have been:
JSON.parseonce (as it can be expensive to run it on large payloads multiple times)O(n)complexity or similar (Avoid nested loops etc.)This has been achieved by parsing the truncated JSON string once, keeping a stack of the JSON tokens, and completing the JSON afterwards in a valid fashion.
Truncated output
For non-JSON content, bodies will simply be truncated to 150k characters, followed by
….For JSON content, we'll add
"~~"a the end of the JSON to indicate where it was truncated. Note that this can take various forms, depending on where the JSON is cut. E.g.:["aa~~"]["aa","bb","~~"]{"aa":"~~"}{"aa":"bb","~~":"~~"}Edge cases
Some edge cases we handle:
"~~". So we never have incomplete numbers, it will replace[1with["~~"].~~added. so["aawill become["aa~~"].["aa"becomes["aa", "~~"].Meta
Instead of the previous
_meta.errorsfield (which is never set anymore), the following_meta.warningscan be set:JSON_TRUNCATED: The field is JSON & was truncatedTEXT_TRUNCATED: The field is plain text & was truncatedINVALID_JSON: We think the field should be JSON but we could not parse it. This can happen if we try to truncate the JSON but something goes wrong - in this case the body sent will be plain string.This is a replacement for #7730.
Closes #7531