mirror of
https://github.com/rclone/rclone.git
synced 2025-12-11 22:14:05 +01:00
rc: make sure fatal errors don't crash rclone - fixes #8955
Before this change, if any code called fs.Fatal(f) then it would stop rclone as designed. However this is not appropriate when using the RC API - we want the error returned to the user. This change turns the fs.Fatal(f) call into a panic which is caught by the RC API handler and returned to the user as a 500 error.
This commit is contained in:
34
fs/log.go
34
fs/log.go
@@ -7,6 +7,9 @@ import (
|
||||
"log/slog"
|
||||
"os"
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
"github.com/rclone/rclone/lib/caller"
|
||||
)
|
||||
|
||||
// LogLevel describes rclone's logs. These are a subset of the syslog log levels.
|
||||
@@ -196,12 +199,42 @@ func Panicf(o any, text string, args ...any) {
|
||||
panic(fmt.Sprintf(text, args...))
|
||||
}
|
||||
|
||||
// Panic if this called from an rc job.
|
||||
//
|
||||
// This means fatal errors get turned into panics which get caught by
|
||||
// the rc job handler so they don't crash rclone.
|
||||
//
|
||||
// This detects if we are being called from an rc Job by looking for
|
||||
// Job.run in the call stack.
|
||||
//
|
||||
// Ideally we would do this by passing a context about but we don't
|
||||
// have one with the logging calls yet.
|
||||
//
|
||||
// This is tested in fs/rc/internal_job_test.go in TestInternalFatal.
|
||||
func panicIfRcJob(o any, text string, args []any) {
|
||||
if !caller.Present("(*Job).run") {
|
||||
return
|
||||
}
|
||||
var errTxt strings.Builder
|
||||
_, _ = errTxt.WriteString("fatal error: ")
|
||||
if o != nil {
|
||||
_, _ = fmt.Fprintf(&errTxt, "%v: ", o)
|
||||
}
|
||||
if args != nil {
|
||||
_, _ = fmt.Fprintf(&errTxt, text, args...)
|
||||
} else {
|
||||
_, _ = errTxt.WriteString(text)
|
||||
}
|
||||
panic(errTxt.String())
|
||||
}
|
||||
|
||||
// Fatal writes critical log output for this Object or Fs and calls os.Exit(1).
|
||||
// It should always be seen by the user.
|
||||
func Fatal(o any, text string) {
|
||||
if GetConfig(context.TODO()).LogLevel >= LogLevelCritical {
|
||||
LogPrint(LogLevelCritical, o, text)
|
||||
}
|
||||
panicIfRcJob(o, text, nil)
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
@@ -211,6 +244,7 @@ func Fatalf(o any, text string, args ...any) {
|
||||
if GetConfig(context.TODO()).LogLevel >= LogLevelCritical {
|
||||
LogPrintf(LogLevelCritical, o, text, args...)
|
||||
}
|
||||
panicIfRcJob(o, text, args)
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
|
||||
@@ -64,6 +64,39 @@ func rcError(ctx context.Context, in Params) (out Params, err error) {
|
||||
return nil, fmt.Errorf("arbitrary error on input %+v", in)
|
||||
}
|
||||
|
||||
func init() {
|
||||
Add(Call{
|
||||
Path: "rc/panic",
|
||||
Fn: rcPanic,
|
||||
Title: "This returns an error by panicing",
|
||||
Help: `
|
||||
This returns an error with the input as part of its error string.
|
||||
Useful for testing error handling.`,
|
||||
})
|
||||
}
|
||||
|
||||
// Return an error regardless
|
||||
func rcPanic(ctx context.Context, in Params) (out Params, err error) {
|
||||
panic(fmt.Sprintf("arbitrary error on input %+v", in))
|
||||
}
|
||||
|
||||
func init() {
|
||||
Add(Call{
|
||||
Path: "rc/fatal",
|
||||
Fn: rcFatal,
|
||||
Title: "This returns an fatal error",
|
||||
Help: `
|
||||
This returns an error with the input as part of its error string.
|
||||
Useful for testing error handling.`,
|
||||
})
|
||||
}
|
||||
|
||||
// Return an error regardless
|
||||
func rcFatal(ctx context.Context, in Params) (out Params, err error) {
|
||||
fs.Fatalf(nil, "arbitrary error on input %+v", in)
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func init() {
|
||||
Add(Call{
|
||||
Path: "rc/list",
|
||||
|
||||
38
fs/rc/internal_job_test.go
Normal file
38
fs/rc/internal_job_test.go
Normal file
@@ -0,0 +1,38 @@
|
||||
// These tests use the job framework so must be external to the module
|
||||
|
||||
package rc_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/rclone/rclone/fs/rc"
|
||||
"github.com/rclone/rclone/fs/rc/jobs"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestInternalPanic(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
call := rc.Calls.Get("rc/panic")
|
||||
assert.NotNil(t, call)
|
||||
in := rc.Params{}
|
||||
_, out, err := jobs.NewJob(ctx, call.Fn, in)
|
||||
require.Error(t, err)
|
||||
assert.ErrorContains(t, err, "arbitrary error on input map[]")
|
||||
assert.ErrorContains(t, err, "panic received:")
|
||||
assert.Equal(t, rc.Params{}, out)
|
||||
}
|
||||
|
||||
func TestInternalFatal(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
call := rc.Calls.Get("rc/fatal")
|
||||
assert.NotNil(t, call)
|
||||
in := rc.Params{}
|
||||
_, out, err := jobs.NewJob(ctx, call.Fn, in)
|
||||
require.Error(t, err)
|
||||
assert.ErrorContains(t, err, "arbitrary error on input map[]")
|
||||
assert.ErrorContains(t, err, "panic received:")
|
||||
assert.ErrorContains(t, err, "fatal error:")
|
||||
assert.Equal(t, rc.Params{}, out)
|
||||
}
|
||||
Reference in New Issue
Block a user