fix: assign target in SignalError.As instead of only checking its type
errors.As calls SignalError.As when it cannot match the concrete type directly, e.g. for `var target SignalError; errors.As(err, &target)`. The previous implementation matched target's type and returned true but never wrote to it, so callers got a successful errors.As result with a zero-valued SignalError and no indication the signal was lost. Add a regression test asserting the target is actually populated.
This commit is contained in:
1 parent
b0e6917276
commit
ae63f3a6f0
2 files changed
+34
-5
No files matched your search
@@ -82,14 +82,18 @@ func (e SignalError) Is(err error) bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// As fixes a design error in the SignalError type, so that errors.As with the
|
// As fixes a design error in the SignalError type, so that errors.As with the
|
||||||
// literal `&SignalError{}` will return true.
|
// literal `&SignalError{}` will return true and, per the errors.As contract,
|
||||||
|
// populate target with the underlying signal.
|
||||||
|
//
|
||||||
|
// errors.As requires target to be a non-nil pointer, so the only target type
|
||||||
|
// that can actually be assigned here is *SignalError.
|
||||||
func (e SignalError) As(target interface{}) bool {
|
func (e SignalError) As(target interface{}) bool {
|
||||||
switch target.(type) {
|
t, ok := target.(*SignalError)
|
||||||
case *SignalError, SignalError:
|
if !ok {
|
||||||
return true
|
|
||||||
default:
|
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
*t = e
|
||||||
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
// ErrSignal is returned by SignalHandler when a signal triggers termination.
|
// ErrSignal is returned by SignalHandler when a signal triggers termination.
|
||||||
|
|||||||
@@ -58,6 +58,31 @@ func TestSignalError(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestSignalErrorAsAssignsTarget verifies that errors.As does not merely
|
||||||
|
// report a match for a value-typed SignalError target, but actually
|
||||||
|
// populates it, per the errors.As contract ("sets target to that error
|
||||||
|
// value"). A prior fix for SignalError's As method (#27) made errors.As
|
||||||
|
// return true for such a target without ever assigning to it, silently
|
||||||
|
// discarding the signal.
|
||||||
|
func TestSignalErrorAsAssignsTarget(t *testing.T) {
|
||||||
|
testc := make(chan os.Signal, 1)
|
||||||
|
ctx := putTestSigChan(context.Background(), testc)
|
||||||
|
|
||||||
|
var rg Group
|
||||||
|
rg.Add(SignalHandler(ctx, os.Interrupt))
|
||||||
|
testc <- os.Interrupt
|
||||||
|
err := rg.Run()
|
||||||
|
|
||||||
|
var target SignalError
|
||||||
|
if want, have := true, errors.As(err, &target); want != have {
|
||||||
|
t.Fatalf("errors.As(err, &target): want %v, have %v", want, have)
|
||||||
|
}
|
||||||
|
|
||||||
|
if want, have := os.Interrupt, target.Signal; want != have {
|
||||||
|
t.Errorf("target.Signal: want %v, have %v (errors.As reported success but did not assign the target)", want, have)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestSignalHandlerNil(t *testing.T) {
|
func TestSignalHandlerNil(t *testing.T) {
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
var rg Group
|
var rg Group
|
||||||
|
|||||||
Reference in new issue
Block a user