Skip to content
This repository has been archived by the owner on Feb 13, 2024. It is now read-only.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MartinBriza
Copy link
Contributor

📒 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.

Reduces a lot of boilerplate code all around the library.
…tion (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 completely insane. Catch-them-all clause 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...
@MartinBriza
Copy link
Contributor Author

Oh, also, I switched the project to C++17 but I forgot why. All the build environments we use should support that though, I hope.

@MartinBriza
Copy link
Contributor Author

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
1 participant