sys4 / tlsrpt

A set of libraries and tools to implement TLSRPT reporting into an MTA and to generate and submit TLSRPT reports.
Other
0 stars 2 forks source link

Request for a function to convert error codes ("res") to text #11

Closed wietse-postfix closed 1 month ago

wietse-postfix commented 3 months ago

As documented, functions in the tlsrpt client library API return a numerical status result that is non-zero if a call failed. Example:

res = tlsrpt_open(args...); 
if (res != 0)  {  /* warn, clean up, etc. */ }

(The res value is based on errno in the case of a system call failure, or it is a tlsrpt library internal error value.)

In Postfix code I would like to log a warning whenever a tlsrpt client library API call fails. For that, I would like to have a simple API that converts a non-zero "res" status to text.

The simplest may be a function that calls strerror() to convert a system error to string, and that converts a tlsrpt client library internal error to string.

Alternatively, there could be a function that tells me if there was a system call error (I should call strerror() to convert the error number to text) or if there was a tlsrpt client library internal error (I should call a not-yet-implemented function to convert such errors to text).

For example, the libc function getaddrinfo() returns EAI_SYSTEM after a system call error (the caller should use strerror(errno) to convert a system error status to text). For a non-system error, the caller should use the libc function gai_strerror() with the result value from the failed getaddrinfo() call.

BLohner commented 2 months ago

There is already a function "tlsrpt_errno_from_error_code" which extracts the errno part.

I can add a "tlsrpt_code_strerror" function that will give descriptive strings in case of library-internal errors (e.g. "No policies where added to the deivery request") or a description of the C-library function and from where it was called iwhen such a call failed (e.g. "fprintf in tlsrpt_init_policy").

Furthermore I think about adding a function "tlsrpt_error_code_has_errno" to distinguish internal errors where errno was not set from C-library errors where the exact reason is given in the errno part of the error code.

While such a disticntion might be useful, the if branch when logging might be cumbersome and could be avoided by adding a "tlsrpt_errno_strerror" function that just wraps strerror but in case of a zero errno returns an empty string instead of "Success". That way logging could look like:

log_printf("tlsrpt library error %d: %s %s", errorcode, tlsrpt_code_strerror(errorcode), tlsrpt_errno_strerror(errorcode));

In case of internal errors the second string would be the empty string. That such an error message ends in a trailing space character is a small disadvantage of this approach, though.

wietse-postfix commented 2 months ago

In my code I need to call one function that converts an error number to text. If libtlsrpt provides two functions that return null or empty depending on the error value, then I can wrap those two functions in a function that returns the result that I need.

BLohner commented 2 months ago

I see. An approach pretty close to what getaddrinfo does would be to provide three functons, one less than in my earlier proposal:

const char* tlsrpt_strerror(errorcode)which gives the full descritive textfor library internal errors or some context for libC errors whose real reason will be given by strerror

bool tlsrpt_error_code_is_internal(errorcode) to decide whether tlsrpt_strerror returned all information or if strerror needs to be called.

int tlsrpt_errno_from_errorcode(errorcode) to extract the errno value to be passed into strerror.

There are no empty strings with this approach and there is no wrapping of strerror, which would be problematic because of thread-safety. That way it is up to the library user to choose a thread-safe alternative of strerror. The tlsrpt_strerror function just returns static strings and is thread-safe like all the other library functions.

wietse-postfix commented 2 months ago

Three functions? Fine.

Wietse
BLohner commented 2 months ago

And only on of them returning a string. The second string will be retrieved directly via strerror, but no fourth function to wrap that call anymore.

I will implement it this way and commit it in a couple of hours.

BLohner commented 2 months ago

I have committed the library yesterday with the two new tlsrpt_strerror and tlsrpt_error_code_is_internal functions. The third function tlsrpt_errno_from_error_code existed already.

wietse-postfix commented 1 month ago

After all this discussion I wonder what was actually implemented. The library has

Are these cases mutually exclusive, i.e.

Or can there be errors that are both internal and external at the same time? How would one find out?

My code goes like this:

if (libtlsrpt_err == 0) {
    // request was successful
    ...
} else if (tlsrpt_error_code_is_internal(libtlsrpt_err)) {
    log("request failed: %s (%d)", tlsrpt_strerror(libtlsrpt_err),
         libtlsrpt_err);
    ...
} else {
    int err = tlsrpt_errno_from_error_code(libtlsrpt_errorcode);
    log("request failed: %s (d)",  mystrerror(err), err); 
    ...
}

Where mystrerror() returns something sensible (instead of returning "Success") when a libc call fails with errno == 0 (yes those things happen).

BLohner commented 1 month ago

Are these cases mutually exclusive, i.e.

* one has to call tlsrpt_strerror() for internal errors (i.e. libstlsrpt),

* one has to call libc strerror()-like functions for external errors (i.e. libc or kernel)?

Almost. They are exclusive.

For internal errors only tlsrpt_strerror() must be called.

For external errors a call to libc strerror()-like functions gives information about what went wrong. In addition to that for external errors a call to tlsrpt_strerror() gives information about where it went wrong.

For example: strerror() will say "Connection refused" and tlsrpt_strerror() will say "TLSRPT error in call to sendto". Sendto is used just once, other calls like fprintf are used in multiple places in the library, then tlsrpt_strerror() will give more details about which libc function was called from which libtlsrpt function, for example:

For better consistency and stability in case of future extensions I think tlsrpt_strerror() should always report libc function and libtlsrpt function, even when there is just one call to that libc function, i.e. "TLSRPT error in call to sendto in finish_delivery_request" instead of just "TLSRPT error in call to sendto".

Or can there be errors that are both internal and external at the same time? How would one find out?

No, errors cannot be internal and external at the same time. But for external errors the full information about what and where needs to be determined by calls as well to libc strerror() and tlsrpt_strerror().

So the last log line of your code could be rewritten to: log("request failed: %s (%d): %s (d)", tlsrpt_strerror(libtlsrpt_err), libtlsrpt_err, mystrerror(err), err);