Skip to content

Conversation

@philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Sep 30, 2020

📜 Description

When capturing an NSError combine the error message of the error.domain and error.code.
The error.localizedDescription is still set in the context of the event as user info. Issues with the
message coming from the domain and code might not be grouped to existing issues which messages
stem from the error.localizedDescription although there origin is the same. For new issues this
is not going to have a big impact on grouping as it is based upon stack trace, exception, and
message.

PR for updating docs: getsentry/sentry-docs#2395

💡 Motivation and Context

The localizedDescription can be in any language and might be confusing for our users when they don't speak this specific language. The domain and code are way more descriptive when an error happened.

💚 How did you test it?

Unit tests and simulator.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the CHANGELOG
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

When capturing an NSError combine the error message of the error.domain and error.code.
The error.localizedDescription is still set in the context of the event as user info. Issues with the
message coming from the domain and code might not be grouped to existing issues which messages
stem from the error.localizedDescription although there origin is the same. For new issues this
is not going to have a big impact on grouping as it is based upon stack trace, exception, and
message.
@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2020

Codecov Report

Merging #750 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #750   +/-   ##
=======================================
  Coverage   92.61%   92.61%           
=======================================
  Files          72       72           
  Lines        3196     3196           
=======================================
  Hits         2960     2960           
  Misses        236      236           
Impacted Files Coverage Δ
Sources/Sentry/SentryClient.m 91.72% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a04b4a...af4f884. Read the comment docs.

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

LGTM

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