-
Notifications
You must be signed in to change notification settings - Fork 181
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
CW-782: Show error report popup without cooldown #1739
base: main
Are you sure you want to change the base?
Conversation
- put _lastOpenedWallet to avoid issues on windows (file is currently open by) - don't throw corruptedWalletsSeed - instead store it inside of secureStorage - await ExceptionHandler.onError calls where possible to makse sure that popup won't be canceled by some UI element - adjust BaseAlertDialog to be scrollable if the text is too long - add ExceptionHandler.resetLastPopupDate - that can be called when we want to show error report screen (bypassing cooldown)
e40177e
to
212e59b
Compare
8fa3993
to
d7327ce
Compare
} | ||
} | ||
|
||
// if all user's wallets are corrupted throw exception | ||
throw error.toString() + "\n\n" + corruptedWalletsSeeds; | ||
secureStorageShared.write(key: "corruptedWalletsSeed", value: corruptedWalletsSeeds); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you write them to secure storage? it will propagate already,
so like this you will add it twice, once from the error string and once more from the secure storage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise the error report may contain the seed - which wouldn't be ideal
lib/src/screens/dashboard/desktop_widgets/desktop_wallet_selection_dropdown.dart
Outdated
Show resolved
Hide resolved
final shouldCopy = await showPopUp<bool?>( | ||
context: navigatorKey.currentContext!, | ||
builder: (context) { | ||
return AlertWithTwoActions( | ||
isDividerExist: true, | ||
alertTitle: S.of(context).error, | ||
alertContent: content, | ||
alertContent: content+"\n"+(seeds??""), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here I believe it would be duplicated, as the content is the error message, which contains the seeds as well
await ExceptionHandler.resetLastPopupDate(); | ||
final err = e.toString() + ((await secureStorageShared.read(key: "corruptedWalletsSeed"))??"unable to retrieve seeds"); | ||
await secureStorageShared.delete(key: "corruptedWalletsSeed"); | ||
await ExceptionHandler.onError(FlutterErrorDetails(exception: err)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same,
same
lib/di.dart
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haven't reviewed this file yet, but double check it as well please
Co-authored-by: Omar Hatem <[email protected]>
5b72be3
to
159f433
Compare
Description
improve exception throwing on broken wallets
Pull Request - Checklist