Skip to content

Commit c43fa6f

Browse files
committed
fix(doctor): time out the S3 storage probe
vikunja doctor hung indefinitely against a blackholed S3 endpoint. 12s is well above a healthy round trip including TLS and SDK retries, well under the OS TCP timeout, and the same order as the existing 5s Redis deadline.
1 parent b0c49e3 commit c43fa6f

1 file changed

Lines changed: 23 additions & 3 deletions

File tree

pkg/doctor/files.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,31 @@ package doctor
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"io/fs"
2324
"os"
2425
"path/filepath"
26+
"time"
2527

2628
"code.vikunja.io/api/pkg/config"
2729
"code.vikunja.io/api/pkg/files"
2830
)
2931

32+
// s3ProbeTimeout bounds every S3 round trip the check makes. An unreachable but
33+
// well-formed endpoint would otherwise block until the OS TCP timeout.
34+
const s3ProbeTimeout = 12 * time.Second
35+
3036
// CheckFiles returns file storage checks.
3137
func CheckFiles() CheckGroup {
3238
fileType := config.FilesType.GetString()
3339

3440
ctx := context.Background()
41+
if fileType == "s3" {
42+
var cancel context.CancelFunc
43+
ctx, cancel = context.WithTimeout(ctx, s3ProbeTimeout)
44+
defer cancel()
45+
}
3546

3647
// Not InitFileHandler: a diagnostic must not create the storage it reports on.
3748
if err := files.InitStorageBackend(ctx); err != nil {
@@ -41,7 +52,7 @@ func CheckFiles() CheckGroup {
4152
{
4253
Name: "Initialization",
4354
Passed: false,
44-
Error: err.Error(),
55+
Error: storageError(ctx, err),
4556
},
4657
},
4758
}
@@ -118,7 +129,7 @@ func checkLocalStorage(ctx context.Context) []CheckResult {
118129
results = append(results, CheckResult{
119130
Name: "Writable",
120131
Passed: false,
121-
Error: err.Error(),
132+
Error: storageError(ctx, err),
122133
})
123134
} else {
124135
results = append(results, CheckResult{
@@ -208,7 +219,7 @@ func checkS3Storage(ctx context.Context) []CheckResult {
208219
results = append(results, CheckResult{
209220
Name: "Writable",
210221
Passed: false,
211-
Error: err.Error(),
222+
Error: storageError(ctx, err),
212223
})
213224
} else {
214225
results = append(results, CheckResult{
@@ -220,3 +231,12 @@ func checkS3Storage(ctx context.Context) []CheckResult {
220231

221232
return results
222233
}
234+
235+
// storageError replaces the raw backend error with an actionable one when our own
236+
// probe deadline fired.
237+
func storageError(ctx context.Context, err error) string {
238+
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
239+
return fmt.Sprintf("S3 endpoint %s did not respond within %s", config.FilesS3Endpoint.GetString(), s3ProbeTimeout)
240+
}
241+
return err.Error()
242+
}

0 commit comments

Comments
 (0)