toggldesktop icon indicating copy to clipboard operation
toggldesktop copied to clipboard

[WIP] Use a enum-based Error type instead of one based on std::string

Open MartinBriza opened this issue 4 years ago • 2 comments

📒 Description

Switch from a std::string-based error type to a class with an enumeration (lib)

This is by no means done (now the UI is getting a ton of Unknown errors) but now it finally compiles and I need to store it somewhere before I go completely mad.

One important thing worth noting is that the way we're doing exception handling now is absolutely insane. Catch-them-all clauses and then searching the reason string for patterns is just about the worst error handling method I've ever seen. I'll need to figure out how to continue from here...

For now, I'm thinking about adding a details field to the Error class and continue using this new class in a way very similar to how the old typedef std::string error was used so we maintain some degree of compatibility with the old solution while we're slowly progressing to a cleaner solution. This is a pretty big undertaking on itself though.

🕶️ Types of changes

  • Bug fix

🔎 Review hints

This will be tough to review. There are a few error types we had before:

  • Errors defined in const.h properly
    • Easy to check if ported right, mostly 1:1, checking was done with == and not std::string::find
  • Errors defined inline in code
    • Usually sane-ish, but very often we have like 5 different messages for the same thing
  • Error strings from exceptions
    • Handled completely wrong, without differentiating between types of exceptions and then searched for substrings somewhere else
    • Not ported over completely yet because the actual resulting error varies heavily depending on the context when it happened
  • Errors returned from the backend
    • There's no real docs for this, some of them are just hardcoded and, again, searched for substrings to pass to the UI somewere further along the way.

MartinBriza avatar Jan 28 '20 18:01 MartinBriza