fix: convert AdditionalMessage param into string type CustomMessage #535
Conversation
Codecov Report
@@ Coverage Diff @@
## master #535 +/- ##
==========================================
+ Coverage 78.65% 78.67% +0.01%
==========================================
Files 23 23
Lines 3158 3160 +2
Branches 207 207
==========================================
+ Hits 2484 2486 +2
Misses 671 671
Partials 3 3
Continue to review full report at Codecov.
|
bcoe
left a comment
There was a problem hiding this comment.
I'm not sure whether to mark this PR as breaking or nonbreaking. Typically a change in an attr type warrants breaking but in this case, this attribute never worked, and is only introduced at the client lib layer...
Am I following that this would been a noop if the user was attempting to set additionalMessage?
My only concern would be that we might break people who were attempting to set additionalMessage, next time they compile -- but, if we think that will be a relatively small number of people, I'm comfortable with the decision to call this a fix.
|
Will closely monitor this repo to see if folks have a lot of issues. I do think this affects a small number of people because this param isn't easily discoverable. It isn't documented unless folks dig into the code comments. Though I am still wary since we don't have concrete usage numbers 😬 |
🤖 I have created a release \*beep\* \*boop\* --- ### [2.0.1](https://www.github.com/googleapis/nodejs-error-reporting/compare/v2.0.0...v2.0.1) (2021-01-14) ### Bug Fixes * convert AdditionalMessage param into string type CustomMessage ([#535](https://www.github.com/googleapis/nodejs-error-reporting/issues/535)) ([ba7d8b0](https://www.github.com/googleapis/nodejs-error-reporting/commit/ba7d8b01b6351354c88a675bfe55910e7a2c0eff)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Fixes #517 🦕
The original intent of additionalMessage was to let users manually override the entire ReportedErrorEvent.message field. Which according to specs, must include the error message & stack trace in the format:
errorMessage + "\n" + stacktrace. Additionally error message must be a string.PR changes:
additionalMessageparam intocustomMessageparam to be clearer about its intended usecustomMessageparam into only string, not object, type. As originally intended per original inline documentation:Tests passing.
Breaking or nonbreaking
I'm not sure whether to mark this PR as breaking or nonbreaking. Typically a change in an attr type warrants
breakingbut in this case, this attribute never worked, and is only introduced at the client lib layer...