Skip to content

Sentry selective log#2967

Open
tddang-linagora wants to merge 3 commits intomainfrom
enhancement/Sentry-selective-log
Open

Sentry selective log#2967
tddang-linagora wants to merge 3 commits intomainfrom
enhancement/Sentry-selective-log

Conversation

@tddang-linagora
Copy link
Copy Markdown
Collaborator

@tddang-linagora tddang-linagora commented Mar 27, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Added robust error handling during app initialization to prevent startup crashes.
    • Made crash reporting more selective: only critical or explicitly tracked events are forwarded.
    • Improved log filtering and severity assignment so true failures are reported as fatal while tracked issues are reported with lower severity.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cd117715-beb0-4455-bbb8-9c658a162b19

📥 Commits

Reviewing files that changed from the base of the PR and between ed0f66b and 1c6910a.

📒 Files selected for processing (1)
  • lib/utils/sentry_init.dart
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/utils/sentry_init.dart

Walkthrough

Sentry logging was changed to filter which entries are sent: SentryLogger.log now sends events only when the entry level is wtf or the entry message contains a substring from SentryTrackedEvents.active. It computes isWtf and isTracked, returns early when neither is true, sends exceptions when entry.error != null, and sends messages with Sentry severity fatal for isWtf or warning for tracked entries. A new SentryTrackedEvents enum was added to list trackable messages. Sentry initialization (sentryInit) is wrapped in try/catch and falls back to running the app while logging any initialization error.

Possibly related PRs

  • TW-2839: Sentry #2840 — Updates Sentry reporting logic and adds tracked-event filtering, modifying the same SentryLogger behavior.

Suggested reviewers

  • dab246
  • nqhhdev
  • 9clg6
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided by the author, but a detailed template exists requiring sections like Ticket, Root cause, Solution, Impact, Testing, Pre-merge, and Resolution information. Provide a complete pull request description following the repository template, including related ticket, solution overview, impact description, and test recommendations.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Sentry selective log' is concise and directly related to the main change: implementing selective logging to Sentry based on log level and tracked event configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch enhancement/Sentry-selective-log

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/utils/logging/sentry_tracked_events.dart`:
- Around line 3-15: The enum SentryTrackedEvents currently defines only
failedToLoadTimeline with isExample: true, which makes the active getter
(SentryTrackedEvents.active) return an empty iterable; change the enum so at
least one event is not marked as an example (e.g., set failedToLoadTimeline to
isExample: false or add a new event like pushHelperCrashed without isExample:
true) so that SentryTrackedEvents.active yields a non-empty collection and
tracked-message forwarding can run.

In `@lib/utils/sentry_init.dart`:
- Around line 34-36: The current catch only handles Exception and misses Errors,
so replace the exception-specific handler in sentryInit (the "on Exception catch
(e, s)" block) with a throwable-wide handler such as "catch (Object e,
StackTrace s)" (or "catch (dynamic e, StackTrace s)"), keep the
Logs().e('sentryInit():', e, s) call, and ensure runApp(app) remains inside that
catch so the fallback app always starts for any thrown Error or Exception.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: acb7aa7b-3764-4ad5-b70a-f11c6c6ea922

📥 Commits

Reviewing files that changed from the base of the PR and between 2db5910 and ed0f66b.

📒 Files selected for processing (3)
  • lib/utils/logging/loggers/sentry_logger.dart
  • lib/utils/logging/sentry_tracked_events.dart
  • lib/utils/sentry_init.dart

@github-actions
Copy link
Copy Markdown
Contributor

This PR has been deployed to https://linagora.github.io/twake-on-matrix/2967

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant