Skip to content

Handle Realm init error with a modal - #761

Merged
tomholub merged 1 commit into
masterfrom
bugfix/issue-759
Oct 24, 2021
Merged

Handle Realm init error with a modal#761
tomholub merged 1 commit into
masterfrom
bugfix/issue-759

Conversation

@ivan-ushakov

@ivan-ushakov ivan-ushakov commented Oct 24, 2021 •
edited by tomholub
Loading

Copy link
Copy Markdown
Contributor

close #759

Tests (delete all except exactly one):

  • Not worth testing

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@ivan-ushakov
ivan-ushakov requested a review from tomholub October 24, 2021 10:54
@ivan-ushakov

ivan-ushakov commented Oct 24, 2021

Copy link
Copy Markdown
Contributor Author

Simulator Screen Shot - iPhone 11 Pro - 2021-10-24 at 13 37 19

tomholub
tomholub reviewed Oct 24, 2021
View reviewed changes
Comment thread FlowCrypt/Functionality/Services/GlobalRouter.swift
Comment on lines +70 to +77
} catch {
let controller = InvalidStorageViewController(
encryptedStorage: storage,
router: self
)
keyWindow.rootViewController = UINavigationController(rootViewController: controller)
keyWindow.makeKeyAndVisible()
}

@tomholub tomholub Oct 24, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should actually use the error description that we caught and also render that to the user.

@ivan-ushakov ivan-ushakov Oct 24, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I found it very long because it contains stack trace.

@tomholub tomholub Oct 24, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's already an error screen, more information is better. The worst we can do is be vague when error happens. You can put it below and make it scrollable, or show the first few hundred characters.

@ivan-ushakov

ivan-ushakov commented Oct 24, 2021

Copy link
Copy Markdown
Contributor Author

Simulator Screen Shot - iPhone 11 Pro - 2021-10-24 at 21 52 44

@ivan-ushakov

ivan-ushakov commented Oct 24, 2021

Copy link
Copy Markdown
Contributor Author

Unfortunately I fail to make this screen using ADK so I switched to pure UIKit.

@tomholub tomholub mentioned this pull request Oct 24, 2021
Closed
@tomholub
tomholub merged commit 606cac4 into master Oct 24, 2021
@tomholub
tomholub deleted the bugfix/issue-759 branch October 24, 2021 19:02
@ivan-ushakov

ivan-ushakov commented Oct 24, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@tomholub
Sorry, forgot to add my changes. I was thinking that we discuss how screen looks like first and after that I will add my changes.

@ivan-ushakov
ivan-ushakov restored the bugfix/issue-759 branch October 24, 2021 19:04
@ivan-ushakov

ivan-ushakov commented Oct 24, 2021

Copy link
Copy Markdown
Contributor Author

OK, I will create new PR

@tomholub

tomholub commented Oct 24, 2021

Copy link
Copy Markdown
Collaborator

When still working on a PR, better mark it as a draft. You can also convert normal PR back to a draft. That way, you know I won't merge it too soon.

tomholub pushed a commit that referenced this pull request Nov 28, 2021
@Kharchevskyi Tom J
issue #761 Rebuild InvalidStorageViewController with Texture (#1109)
72bdc0d 
* Rebuild InvalidStorageViewController with Texture

* Remove redundant MainActor

Co-authored-by: Tom J <tom@holub.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomholub tomholub tomholub approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

handle Realm init error with a modal

2 participants

@ivan-ushakov @tomholub