Manager: more gracefull handle SQLite "interrupted (9)" error

Wrap the SQLite error "interrupted (9)". That error is (as far as I
could figure out) caused by the context being closed. Unfortunately
there is no wrapping of the underlying context error, so it's not
possible to determine whether it was due to a 'deadline exceeded' error
or another cancellation cause (like upstream HTTP connection closing).

Primarily this makes a rather unreliable unit test properly reliable.
The code under test could return either `context.DeadlineExceeded` or
the "interrupted (9)" error (GORM + SQLite doesn't reliably chose one or
the other), and now this is cleanly tested for.
This commit is contained in:
Sybren A. Stüvel 2024-05-28 15:24:17 +02:00
parent 572089f13b
commit 7fd8eca8d9
2 changed files with 26 additions and 1 deletions

@ -17,6 +17,13 @@ var (
ErrWorkerTagNotFound = PersistenceError{Message: "worker tag not found", Err: gorm.ErrRecordNotFound}
ErrDeletingWithoutFK = errors.New("refusing to delete a job when foreign keys are not enabled on the database")
// ErrContextCancelled wraps the SQLite error "interrupted (9)". That error is
// (as far as Sybren could figure out) caused by the context being closed.
// Unfortunately there is no wrapping of the context error, so it's not
// possible to determine whether it was due to a 'deadline exceeded' error or
// another cancellation cause (like upstream HTTP connection closing).
ErrContextCancelled = errors.New("context cancelled")
)
type PersistenceError struct {
@ -57,6 +64,12 @@ func wrapError(errorToWrap error, message string, format ...interface{}) error {
formattedMsg = message
}
// Translate the SQLite "interrupted" error into something the error-handling
// code can check for.
if errorToWrap.Error() == "interrupted (9)" {
errorToWrap = ErrContextCancelled
}
return PersistenceError{
Message: formattedMsg,
Err: errorToWrap,

@ -5,6 +5,7 @@ package persistence
import (
"context"
"errors"
"testing"
"time"
@ -412,6 +413,17 @@ func TestSummarizeWorkerStatusesTimeout(t *testing.T) {
// Test the summary.
summary, err := f.db.SummarizeWorkerStatuses(subCtx)
assert.ErrorIs(t, err, context.DeadlineExceeded)
// Unfortunately, the exact error returned seems to be non-deterministic.
switch {
case errors.Is(err, context.DeadlineExceeded):
// Good!
case errors.Is(err, ErrContextCancelled):
// Also good!
case err == nil:
t.Fatal("no error returned where a timeout error was expected")
default:
t.Fatalf("unexpected error returned: %v", err)
}
assert.Nil(t, summary)
}