browserpass-extension icon indicating copy to clipboard operation
browserpass-extension copied to clipboard

Improve how we present error messages

Open max-baz opened this issue 6 years ago • 5 comments

Native app sends detailed error messages in JSON format, but in the extension we either only show the message (omitting everything else), or print out a raw JSON (like on the options page).

We can do better, maybe show some kind of table with message, details, version of the native app, etc. This needs to be done in popup, options during init and options on save.

Also make sure we log full error details in console.

max-baz avatar Mar 11 '19 11:03 max-baz

I thought a bit about how one might improve the error handling while I was working on the Chrome OS port of browserpass-native.

In order to cleanly separate anticipated extension errors (errors occurring within browserpass-extension for which we currently provide a custom message), host errors (errors encoded in an error response sent by a browserpass-native implementation) and any other kind of error, I would propose to introduce two custom error classes with special features: HostError and ExtensionError.

Both custom error types would log themselves to the background console (which can be accessed from anywhere via chrome.runtime.getBackgroundPage()) when they are created, meaning that logging logic could be simplified and the background page's console would be the only place to look for errors. The custom errors would still be passed up the call chain and forwarded through extension messaging in order to reach the point where they can be displayed in the UI.

HostError would know the defined error codes and their generic descriptions, as well as the expected params. It would log all the information to the console in a structured way, using console.table for the params. It would also offer deserialization from JSON to allow regenerating a HostError that has passed through extension messaging, and would provide a function that renders the error details as a pretty HTML table.

ExtensionError would be much more basic, as it is only needed to simplify logging logic and to distinguish anticipated errors from those that are not yet covered by proper error messages. It would consists solely of a message and could just be reduced to a string when passing through extension messaging.

Whenever anyone encounters an error in the console which is not of one of the two types, we would want to add an appropriate try...catch for it and throw an ExtensionError with a message that makes sense to end users.

What do you think? If we can agree on a concept I would offer to implement it.

fmeum avatar Apr 30 '19 20:04 fmeum

Your proposal sounds quite reasonable to me, I would only ask for a single UI for both types of errors, i.e. both of them would perhaps render a pretty HTML table with error details, but ExtensionError would only contain a single row, error message.

We might actually consider adding more rows to that table, for example to display browser extension version and host app version directly there, so that users don't need to dig up that info when creating a bug report, a simple screenshot of that error will tell us everything.

I'd be super happy if you could help implementing this, if you don't mind my only ask would be to wait with merge until I implement #61 - it's quite big change, with new error types, with new popup window, which means more places to handle and display errors. I'm currently in progress, will try to prioritize this as much as I can. If you have time to start the implementation now, of course you are welcome to begin the work, I'd only ask you to rebase when #61 is in master.

max-baz avatar Apr 30 '19 21:04 max-baz

Apologies for the big delay @FabianHenneke, I don't find the time to finish #61 so if you want to proceed with this your proposal, please go ahead.

max-baz avatar Aug 31 '19 17:08 max-baz

Should also address double "Error: Error" while we are at it :) ref #163

max-baz avatar Aug 31 '19 17:08 max-baz

I'm sorry, but I'm somewhat busy with other things now and will not be able to improve the error handling. I will try to get #127 done though.

fmeum avatar Sep 01 '19 06:09 fmeum