fix: abridge call graph creation error messages in analytics #1279
Conversation
ghost
requested review from
| return msg; | ||
| } | ||
| const e = ellipsis || ' ... '; | ||
| const toKeep = (maxLen - e.length) / 2; |
There was a problem hiding this comment.
I think toKeep can be a not whole number
There was a problem hiding this comment.
I know. But it seems to round it when used in slice.
There was a problem hiding this comment.
I think it would be better to round it before, less JS magic
| if (msg.length <= maxLen) { | ||
| return msg; | ||
| } | ||
| const e = ellipsis || ' ... '; |
There was a problem hiding this comment.
Maybe have it as an optional param on line 4?
ellipsis = '...': string,
There was a problem hiding this comment.
Is that supported by TypeScript?
| analytics.add('callGraphError', err.innerError.toString()); | ||
| analytics.add( | ||
| 'callGraphError', | ||
| abridgeErrorMessage( |
There was a problem hiding this comment.
This should go also to the test file?
There was a problem hiding this comment.
Testing that method would be quite involved, and the functional tests for abridging the string should cover a reasonable number of cases.
| analytics.add('callGraphError', err.innerError.toString()); | ||
| analytics.add( | ||
| 'callGraphError', | ||
| abridgeErrorMessage( |
There was a problem hiding this comment.
I think it would be nice to say why we are doing this trimming with a comment
There was a problem hiding this comment.
That is pretty self-explanatory from the code, no? E.g. ANALYTICS_PAYLOAD_MAX_LENGTH
|
🎉 This PR is included in version 1.364.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this PR do?
Since the call graph error messages can get pretty big, abridge them to something more manageable. This keep the beginning, and end of the message since, presumably, that's where the most important details are.