Skip to content

MS-856 Better alert screen tracking#1630

Merged
luhmirin-s merged 2 commits into
mainfrom
feature/MS-856-better-alert-screen-tracking
Mar 30, 2026
Merged

MS-856 Better alert screen tracking#1630
luhmirin-s merged 2 commits into
mainfrom
feature/MS-856-better-alert-screen-tracking

Conversation

@luhmirin-s
Copy link
Copy Markdown
Contributor

@luhmirin-s luhmirin-s commented Mar 30, 2026

JIRA ticket
Will be released in: 2026.2.0

Notable changes

  • Added alert event ID to user properties for a bit of traceability
  • Tracking a non-fatal when "Unexpected error" alert is displayed to gather logs.

I also have reviewed all of the cases where alert screen is triggered and it seems that if there is an exception, we are at least logging it. This additional dedicated exception should be simpler to trace across

Testing guidance

  • There isn't anything to test really, this is just logging.

Additional work checklist

  • Effect on other features and security has been considered
  • Design document marked as "In development" (if applicable)
  • External (Gitbook) and internal (Confluence) Documentation is up to date (or ticket created)
  • Test cases in Testiny are up to date (or ticket created)
  • Other teams notified about the changes (if applicable)

@luhmirin-s luhmirin-s requested review from a team, BurningAXE, TristramN, alex-vt, alexandr-simprints, meladRaouf and ybourgery and removed request for a team March 30, 2026 10:57
@cla-bot cla-bot Bot added the ... label Mar 30, 2026
@luhmirin-s luhmirin-s force-pushed the feature/MS-856-better-alert-screen-tracking branch from c458372 to 095a7c9 Compare March 30, 2026 11:18
vm.saveAlertEvent(it)

// Report non-fatal to keep the logs for investigation.
// Called after saving the event to have the event ID in user properties for easier correlation.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the event is saved in a coroutine, there isn't a guarantee that it has been completed by the time this is reached. Plus, this logic is better placed in the VM anyway. So, can we move in saveAlertEvent()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wanted it to be as close to the fragment as possible to have cleaner stack trace and I completely missed the launch part. Moved into the coroutine to ensure that ID is always tracked.

@luhmirin-s luhmirin-s force-pushed the feature/MS-856-better-alert-screen-tracking branch from 095a7c9 to 1ba72eb Compare March 30, 2026 11:49
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
60.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link
Copy Markdown
Contributor

@BurningAXE BurningAXE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much simpler that we (or at least I) thought!

@luhmirin-s luhmirin-s merged commit e582871 into main Mar 30, 2026
12 of 14 checks passed
@luhmirin-s luhmirin-s deleted the feature/MS-856-better-alert-screen-tracking branch March 30, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants