-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(release-health): Prevent sending terminal status session updates #3701
Conversation
size-limit report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can simplify the errors calc, and then a comment about including SessionStatus.Exited
as a terminal state.
Drops sending session updates for sessions that are already in terminal states and caps the number of errors for session at 1
fe53dd9
to
1534e1e
Compare
packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js
Outdated
Show resolved
Hide resolved
function validateSessionCountFunction(sessionCounts) { | ||
process.on('exit', exitCode => { | ||
const { sessionCounter, expectedSessions } = sessionCounts; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function validateSessionCountFunction(sessionCounts) { | |
process.on('exit', exitCode => { | |
const { sessionCounter, expectedSessions } = sessionCounts; | |
function validateSessionCountFunction({ sessionCounter, expectedSessions }) { | |
process.on('exit', exitCode => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't work, because this way you are only passing the initial values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right. References 🙄
This comment has been minimized.
This comment has been minimized.
…transport * upstream/master: (29 commits) ref: Always use lowercase files (getsentry#3742) feat: Make dedupe integration default for browser (getsentry#3730) ref(ember): Allow initing Ember without config entry (getsentry#3745) fix(serverless): wrapEventFunction does not await for async code (getsentry#3740) Metrics: Tag CLS elements (getsentry#3734) feat: Add Next.js 11 to supported peer dependencies list (getsentry#3711) test: Run integration tests for Next 10/11 and Webpack 4/5 matrix (getsentry#3741) fix: Correctly limit Buffer requests (getsentry#3736) Whoops. Remove pinned node version from package.json ref: Introduce test runner for node session health tests (getsentry#3728) fix: Prevent circular structure serialization in events (getsentry#3727) ref(node): Update Node manual tests and test for sessionCount (getsentry#3726) ref(ember): Update scenarios and remove a few to speed up tests (getsentry#3720) docs: Fix typos (getsentry#3716) fix(ember): Fix ember test flake (getsentry#3719) release: 6.7.2 ci: fix ember flaky test (getsentry#3718) misc: changelog for release 6.7.2 (getsentry#3717) fix(release-health): Prevent sending terminal status session updates (getsentry#3701) ref: Make beforeSend more strict (getsentry#3713) ...
This PR:
Crashed, Abnormal, Exited
startSession
userAgent
onstartSession
from global object and sets theuserAgent
attributeFixes #3629