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

Display an abstracted message for Sargon errors #1179

Merged
merged 4 commits into from
Jun 20, 2024

Conversation

danvleju-rdx
Copy link
Contributor

@danvleju-rdx danvleju-rdx commented Jun 20, 2024

Jira ticket: ABW-3454

Relates to #168

Description

  • Display an abstracted message for Sargon errors with the option to email support and include more debugging info
  • For debug mode, show an error code and the detailed error message from Sargon instead of the abstracted message

Notes

If you want to simulate this error from Sargon, you can scan this invalid QR code:

Screenshot 2024-06-20 at 15 50 42

Screenshot

Release example Debug example
IMG_0014 IMG_0013
Email body
IMG_0015

@danvleju-rdx danvleju-rdx changed the title Update sargon errors Display an abstracted message for Sargon errors Jun 20, 2024
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX left a comment

Choose a reason for hiding this comment

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

nice, LGTM!

Comment on lines 22 to 25
var body = await buildBody()
if let additionalBodyInfo {
body.append("\n\(additionalBodyInfo)")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would send the additionalBodyInfo to buildBody()

Comment on lines +1530 to +1535
/// Email Support
public static let emailSupportButtonTitle = L10n.tr("Localizable", "error_emailSupportButtonTitle", fallback: "Email Support")
/// Please email support to automatically provide debugging info, and get assistance.
/// Code: %@
public static func emailSupportMessage(_ p1: Any) -> String {
return L10n.tr("Localizable", "error_emailSupportMessage", String(describing: p1), fallback: "Please email support to automatically provide debugging info, and get assistance.\nCode: %@")
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to merge radixdlt/apps-localization-src#411 before otherwise next time Swiftgen runs, these will be deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

merged

Copy link
Contributor

Choose a reason for hiding this comment

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

As a rule, normal PRs should never touch Localizable.strings or L10n.generated.swift, that should only be done in the automatic PRs from the crowdin integration.

@danvleju-rdx danvleju-rdx merged commit 9e5c192 into main Jun 20, 2024
6 checks passed
@danvleju-rdx danvleju-rdx deleted the update-sargon-errors branch June 20, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants