Skip to content
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

Add markUnhandledAsFatal to SentryOptions #1720

Closed

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Nov 13, 2023

📜 Description

Add markUnhandledAsFatal option to SentryOptions. When disabled the level for automatically tracked unhandled evetns will be set to error instead of fatal/

💡 Motivation and Context

Relates to #1624

💚 How did you test it?

Unit tets

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

Copy link
Contributor

github-actions bot commented Nov 13, 2023

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 6799913

Copy link

codecov bot commented Nov 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9b28718) 91.12% compared to head (6799913) 90.30%.

Additional details and impacted files
@@                Coverage Diff                @@
##           release/8.0.0    #1720      +/-   ##
=================================================
- Coverage          91.12%   90.30%   -0.82%     
=================================================
  Files                175      179       +4     
  Lines               5690     5850     +160     
=================================================
+ Hits                5185     5283      +98     
- Misses               505      567      +62     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@denrase denrase marked this pull request as ready for review November 13, 2023 10:26
@@ -2,6 +2,10 @@

## 8.0.0

### Features

- Add `markUnhandledAsFatal` to `SentryOptions` ([#1720](https://github.com/getsentry/sentry-dart/pull/1720))
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 🚫 The changelog entry seems to be part of an already released section ## 8.0.0.
    Consider moving the entry to the ## Unreleased section, please.

Copy link
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

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

lgtm 👍

Any particular reason why this is in v8?

@ueman
Copy link
Collaborator

ueman commented Nov 13, 2023

lgtm 👍

Any particular reason why this is in v8?

It would change the metrics significantly in the dashboards in a minor version.

@denrase
Copy link
Collaborator Author

denrase commented Nov 13, 2023

@buenaflor Also the naming of the flag is in reference to unhandled error, which is also new in v8.

@denrase
Copy link
Collaborator Author

denrase commented Nov 13, 2023

@buenaflor @ueman @feinstein I'm always unsure regarding naming. Is the it clear enough from the options name what it's intent is?

@feinstein
Copy link

Me too hehe, but I think it's ok, I always try to make my booleans start with is, has, should, but I think it's clear its intent.

@denrase denrase marked this pull request as draft November 20, 2023 09:36
@denrase
Copy link
Collaborator Author

denrase commented Nov 20, 2023

Converting back to draft as we decided to base this on the main branch and update naming accordingly.

@denrase denrase changed the base branch from release/8.0.0 to main November 20, 2023 09:38
@denrase denrase changed the base branch from main to release/8.0.0 November 20, 2023 09:38
@denrase
Copy link
Collaborator Author

denrase commented Nov 20, 2023

Closed in favour of #1738

@denrase denrase closed this Nov 20, 2023
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.

5 participants