Help about the new error types

[dodmi]

Hi @lieser ,
I guess, I need your assistance again.
In the 5.x branch, you removed the DKIM_InternalError and added DKIM_TempError and DKIM_Error

I‘d like to backport this to the 2.x branch - to have the error types consistent.

While DKIM_Error should be an „general DKIM error“, as I understand, you replaced DKIM_InternalError with plain Error and are using Error also in various places.
In which cases should I use Error and when DKIM_Error?

DKIM_TempError should mean „temporary DKIM signature error“; therefore I probably need to use it for any DNS problem, even if it’s not a signature problem. But fixing DNS might help fixing verification. Am I right?

(Especially in some places you’re checking for instanceof DKIM_InternalError to decide, if it’s a TEMPFAIL.)

DKIM_InternalError, DKIM_TempError and DKIM_Error all take different parameters. While it’s easy to build a string for DKIM_Error beforehand and switching the params (and put the error message in an array) for DKIM_TempError - should I do it?

I‘d implement the new errors more like DKIM_InternalError and not like the new ones.

Could you guide me a little?

Kind regards

[dodmi]

Here's my first proposal:
#413

[lieser]

DKIM_SigError

• User visible errors that occur during DKIM signature verification.
• Take localized strings and its potential parameters as arguments.
  • Note that unlike in the 2.x branch on master the errors only get translated later.
• Are considered permanent failures and will be e.g. saved if result saving is enabled.

DKIM_TempError

• User visible errors that are considered temporary failures
• Take localized strings and its potential parameters as arguments.
  • Note that unlike in the 2.x branch on master the errors only get translated later.
• Are not saved if result saving is enabled
• This are basically only DNS lookup errors.

DKIM_Error

• Errors that are not directly shown to the user.
• Are still expected to potentially occur if bad input is given (e.g. wrongly formatted e-mail).
• Take an arbitrary error explanation as argument that will only show up in the log not the user interface.

Error

• Errors that should normally not occur, and indicate a potential programming error.
• Take an arbitrary error explanation as argument that will only show up in the log not the user interface.
• Are also used for simplicity in all the experimental code.

you replaced DKIM_InternalError with plain Error and are using Error also in various places.
In which cases should I use Error and when DKIM_Error?

The main reason for the change was that I got the feeling I used the old DKIM_InternalError for multiple different kind of errors that should also be handled different, which caused problems.
I tried to describe the different used errors above in more detail.

DKIM_TempError should mean „temporary DKIM signature error“; therefore I probably need to use it for any DNS problem,
even if it’s not a signature problem.
But fixing DNS might help fixing verification. Am I right?

In the unlikely case someone publishes a wrong DNS key then yes, this could potentially be fixed by changing the DNS error.
I still consider this a permanent error.
In most cases this will not be a "wrong" key. But just an administrator following in my opinion bad practice, and reusing a selector and publishing a new key under it.

What I consider temporary errors here are problems getting the DNS entries from the DNS server. Not a potentially wrongly configured DKIM setup.

[dodmi]

Thank you for the detailed explanation. I guess, I'll have to look over my pull request again :)