Skip to content

feat: Allow error event value (description) to be customised #2120

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

Conversation

lukeredpath
Copy link
Contributor

@lukeredpath lukeredpath commented Sep 2, 2022

📜 Description

Currently errors are hardcoded to use "Code: xxx" using the error's code which make for a very unhelpful display in the Sentry error list.

To alleviate this, check for the presence of a custom description in the error's userInfo dictionary using the NSDebugDescriptionErrorKey

If no custom description is set, the old behaviour is retained of using "Code: xxx".

💡 Motivation and Context

Makes it easier to identify errors in the error list in the Sentry dashboard.

Fixes #1442.

💚 How did you test it?

Integrated the branch into our own app and ran it, generated some errors and verified that the error was displayed correctly.

We have been running this branch in production in the Community app since I opened this PR without issue.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

Sorry, something went wrong.

lukeredpath and others added 8 commits February 9, 2022 18:00
If one isn't available, the system will automatically default to
one based on the domain and code.
The system default is otherwise a bit long.
This avoids the problem of localized descriptions being locale-dependent.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Just one small note, apart from that LGTM. Thanks a lot, @lukeredpath 👏

And don't worry about CI. The failing jobs don't work for contributors and we didn't fix them yet.

Can you please open a PR to the docs? What you have in lukeredpath/sentry-docs#1 looks fine. I think it's worth mentioning for which SDK versions this feature will be available.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot 😁

@philipphofmann
Copy link
Member

@lukeredpath, I can't merge because there are some conflicts that must be resolved
Screen Shot 2022-09-06 at 17 58 36

@lukeredpath
Copy link
Contributor Author

@philipphofmann resolved!

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM

@philipphofmann philipphofmann merged commit 11546dd into getsentry:master Sep 7, 2022
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.

Error message is not included for Swift errors
3 participants