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

Add error reporting #71

Merged
merged 1 commit into from
Jun 22, 2018
Merged

Add error reporting #71

merged 1 commit into from
Jun 22, 2018

Conversation

sabbaka
Copy link

@sabbaka sabbaka commented Jan 5, 2018

Fix #67

@spbnick
Copy link
Member

spbnick commented Jan 17, 2018

Thank you, Kyrill. This looks alright in general. However, I think the error panel should be a part of the player somehow.

We also need to decide if loading errors would be recoverable or not. In principle, a loading/parsing error can happen in the middle of the recording (e.g. due to journal corruption, a network issue, or tlog bug while recording), and it would still be useful to allow watching whatever was already recorded. If we do that, then it should be possible to dismiss the error, the way you implemented it.

However, it should still be obvious that there was an error and it should be possible to recall that error message, if needed. How about we have the error replace the controls bar when it happens, but after closing it, the control bar would have that same error icon on the right, and clicking that icon would bring the error back? Since the loading stops on first error, we should never need to display more than one and that should be sufficient. BTW, it would be good to also explain what the effect the error has. E.g. add the text "Error, recording loading stopped: " before the error text.

Do you think you can implement this, and also make the player handle errors properly, i.e. allow playing sessions loaded incompletely? You can simulate that condition by e.g. adding this to parseMessage:

        if (pos > 5000) {
            throw "simulated error";
        }

Another thing we can do to make this more obvious is to normally have a loading indicator in place of that error icon, on the right of the buttons, while we're still loading, then have another indicator after we reached the end of the recording and are polling for new journal entries, and then those two would be exclusive with the error icon, giving the user the hint that the loading/waiting is no longer happening.

@sabbaka sabbaka merged commit cb750db into Scribery:scribery Jun 22, 2018
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.

2 participants