Log job retries as warnings instead of errors

[begelundmuller]

First of all, thank you for building River, it's a wonderful library!

This PR changes the error reporting to log retryable errors at warn level, reserving the error level for jobs that fail after reaching MaxAttempts.

This is desirable for my use case because I consider error-level logs as requiring human inspection/intervention, which is only needed for jobs that fail even after retrying. However I realize this may not be a practice everyone shares, so please let me know if you would prefer another approach here.

[bgentry]

I think I'm in favor of this change. These are cases where the system is operating as expected and nothing is wrong per se, it's just that the job itself had an error (which may be totally normal depending on the job). Bundling that together with actual River errors results in a lot of noise and makes it harder to distinguish actual issues.

Thoughts, @brandur ?

[brandur]

Yeah, works for me. It does seem to me to be a little on the odd side that these errors are warnings. Not to say they should be info-level per se either, but you could make a similar argument about warnings as you could about errors — i.e. that warnings should be somewhat actionable as well because they represent something that shouldn't be happening.

There's a test failure in one of the example tests. Can you take a look at that? Also, can you add a changelog? Thx.

[bgentry]

Yeah, works for me. It does seem to me to be a little on the odd side that these errors are warnings. Not to say they should be info-level per se either, but you could make a similar argument about warnings as you could about errors — i.e. that warnings should be somewhat actionable as well because they represent something that shouldn't be happening.

Yeah, this is a tough balance. With job errors in particular, users have a great option to error handlers to do whatever logging they want, whereas aside from adjusting the log level they can't control what River will output. For that reason I think we can safely exclude regular errors, potentially even discarded jobs if we want to, and keep the noise down for "expected" errors from the library's perspective.

[begelundmuller]

There's a test failure in one of the example tests. Can you take a look at that? Also, can you add a changelog? Thx.

Done!

[brandur]

Yeah, this is a tough balance. With job errors in particular, users have a great option to error handlers to do whatever logging they want, whereas aside from adjusting the log level they can't control what River will output. For that reason I think we can safely exclude regular errors, potentially even discarded jobs if we want to, and keep the noise down for "expected" errors from the library's perspective.

@bgentry Yep agreed.

Done!

@begelundmuller Excellent. Thanks!

[bgentry]

@begelundmuller thank you for this! Would you also be able to add yourself to the CLA when you have a moment? https://github.com/riverqueue/rivercla