deltachat / deltachat-desktop

Email-based instant messaging for Desktop.
GNU General Public License v3.0
915 stars 166 forks source link

helper function for user visible errors on exception in async action #3883

Open Simon-Laux opened 4 months ago

Simon-Laux commented 4 months ago

we should make a helper function for this:

Some hook like

function useCallbackWithUserAlert(cb, dependencies, alertMessage) {
 const openAlertDialog = useAlertDialog()
 return useCallback(()=>{
  return async (...args) => {
  try {
    await cb(...args)
  } catch (error) {
    log.error(alertMessage, error)
    openAlertDialog({
      message: alertMessage + error && error.message ?? error.toString()
    })
  }
 }, [openAlertDialog,...dependencies])
}

that we could use in these places:

const startChatWithContact = useCallbackWithUserAlert(async (addr: string) => {}, [])

_Originally posted by @Simon-Laux in https://github.com/deltachat/deltachat-desktop/pull/3840#discussion_r1620768005_

WofWca commented 2 weeks ago

There could be a way to use a callback for all unhanded promise rejections.

But the proposed idea seems fine.

Though I am not sure how many places there are where this is applicable.

Simon-Laux commented 2 weeks ago

There could be a way to use a callback for all unhanded promise rejections.

Those are already logged. I think opening a dialog for every one of them could be annoying if we have some bug in some method that gets called often, say in maybe_network or similar. In those cases DC might become unusable because it shows many of those dialogs.

I think it makes most sense to do what I proposed to control when a dialog will be shown. Sure requires more discipline by developers to not forget it, but does not have the risk of making DC unusable by opening tons of alert dialogs.

Though I am not sure how many places there are where this is applicable.

many places where we call core in response to a user action -> mute chat for would be easy example.

maxphilippov commented 1 week ago

Those are already logged. I think opening a dialog for every one of them could be annoying if we have some bug in some method that gets called often, say in maybe_network or similar. In those cases DC might become unusable because it shows many of those dialogs.

Maybe it's better then as a simple user-friendly log somewhere in the UI (have a simple message but store trace log so you can copy it to clipboard and send as report)? It can just be an icon with a number of registered errors in the last couple of minutes or so, so you're aware that something wrong happened, but you don't see messages unless you open the log explicitly. And we need a way to easily demote any single one to just logging to dev console, so when we have some persistent problem we don't bother users to much before we fix it.

Simon-Laux commented 1 week ago

If the user triggered an action they can get an error/alert dialog when it fails. When some method throws not triggered by user action (all other uncaught exception) it just gets logged how it's currently logged.

I don't understand what speaks against this solution.