Wrap all heartbeats methods in try/catch #8425
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 27eba97 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1This report is too large (150,977 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.Test Logs |
DellaBitta
approved these changes
Aug 14, 2024
Merged
GogoVega
reviewed
Aug 17, 2024
| // service, not the browser user agent. | ||
| const agent = platformLogger.getPlatformInfoString(); | ||
| const date = getUTCDateString(); | ||
| console.log('heartbeats', this._heartbeatsCache?.heartbeats); |
There was a problem hiding this comment.
hi @hsubox76
this is showing in our production logs now, can you please remove it
it took me a while to see where is it coming from
I updated many dependencies and wasn't sure which one caused these logs
There was a problem hiding this comment.
After updating Firebase to version 10.13.0, I'm seeing a console log message that says heartbeats undefined. It would be great if the log could be removed!
There was a problem hiding this comment.
I can't update to 10.13.0 because of this as it adds a lot of noise to my frontend monitoring logs.
EDIT: they'll release a fix soon: #8436 (comment)
There was a problem hiding this comment.
@GogoVega @robertIsaac @Tosters @BenaymS @gustavopch
This was removed in 10.13.1
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Heartbeat code should never throw and interfere with developer app operation. This PR wraps the two main heartbeat methods,
triggerHeartbeat()andgetHeartbeatsHeader()in try/catch blocks that hopefully catch unexpected errors in uncommon environments and logs aconsole.warn()instead.I've been told in previous issues that thrown errors cause a lot of noise in Sentry logs (plus interrupting the app) while a
console.warn()does not clog up Sentry logs, so I think this is a good compromise to avoid swallowing any fixable errors that we might want to know about.Fixes #8407