Skip to content

Commit f217c48

Browse files
acabarbayeacabarbaye
authored
⚙️ [parallelisation] Report the context cancellation cause in the related error to provide more context (#727)
<!-- Copyright (C) 2020-2022 Arm Limited or its affiliates and Contributors. All rights reserved. SPDX-License-Identifier: Apache-2.0 --> ### Description - this is to make use of the feature introduced in go 1.20 https://pkg.go.dev/context#Cause which should help determining what created the cancellation of a context. ### Test Coverage <!-- Please put an `x` in the correct box e.g. `[x]` to indicate the testing coverage of this change. --> - [x] This change is covered by existing or additional automated tests. - [ ] Manual testing has been performed (and evidence provided) as automated testing was not feasible. - [ ] Additional tests are not required for this change (e.g. documentation update).
1 parent 50426c0 commit f217c48

File tree

6 files changed

+116
-42
lines changed

6 files changed

+116
-42
lines changed

‎changes/20251015114355.bugfix‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
:gear: `[parallelisation]` Report the context cancellation cause in the related error to provide more context

‎utils/filesystem/files_test.go‎

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"context"
1010
"fmt"
1111
"io"
12-
"net"
1312
"os"
1413
"path/filepath"
1514
"reflect"
@@ -23,7 +22,6 @@ import (
2322
"github.com/spf13/afero"
2423
"github.com/stretchr/testify/assert"
2524
"github.com/stretchr/testify/require"
26-
"golang.org/x/sys/unix"
2725

2826
"github.com/ARM-software/golang-utils/utils/collection"
2927
"github.com/ARM-software/golang-utils/utils/commonerrors"
@@ -549,9 +547,6 @@ func TestConvertPaths(t *testing.T) {
549547
}
550548

551549
func TestIsFile(t *testing.T) {
552-
if platform.IsWindows() {
553-
t.Skip("Windows doesn't have features such as named pipes or unix sockets")
554-
}
555550
for _, fsType := range FileSystemTypes {
556551
t.Run(fmt.Sprint(fsType), func(t *testing.T) {
557552
fs := NewFs(fsType)
@@ -567,38 +562,6 @@ func TestIsFile(t *testing.T) {
567562
assert.True(t, b)
568563
})
569564

570-
t.Run("special file", func(t *testing.T) {
571-
if fsType == InMemoryFS {
572-
t.Skip("In-memory file system won't have hardware devices or special files")
573-
}
574-
575-
b, err := fs.IsFile("/dev/null")
576-
require.NoError(t, err)
577-
assert.True(t, b)
578-
579-
fifoPath := filepath.Join(tmpDir, faker.Word())
580-
require.NoError(t, err)
581-
defer func() { _ = fs.Rm(fifoPath) }()
582-
err = unix.Mkfifo(fifoPath, 0666)
583-
require.NoError(t, err)
584-
b, err = fs.IsFile(fifoPath)
585-
require.NoError(t, err)
586-
assert.True(t, b)
587-
err = fs.Rm(fifoPath)
588-
require.NoError(t, err)
589-
590-
socketPath := filepath.Join(tmpDir, faker.Word())
591-
require.NoError(t, err)
592-
defer func() { _ = fs.Rm(socketPath) }()
593-
l, err := net.Listen("unix", socketPath)
594-
require.NoError(t, err)
595-
defer func() { _ = l.Close() }()
596-
b, err = fs.IsFile(socketPath)
597-
require.NoError(t, err)
598-
assert.True(t, b)
599-
err = fs.Rm(socketPath)
600-
require.NoError(t, err)
601-
})
602565
})
603566
}
604567
}

‎utils/filesystem/files_unix_test.go‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
//go:build !windows
2+
// +build !windows
3+
4+
package filesystem
5+
6+
import (
7+
"fmt"
8+
"net"
9+
"path/filepath"
10+
"testing"
11+
12+
"github.com/go-faker/faker/v4"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
"golang.org/x/sys/unix"
16+
)
17+
18+
func TestIsUnixFile(t *testing.T) {
19+
for _, fsType := range FileSystemTypes {
20+
t.Run(fmt.Sprint(fsType), func(t *testing.T) {
21+
fs := NewFs(fsType)
22+
23+
tmpDir := t.TempDir()
24+
25+
t.Run("normal file", func(t *testing.T) {
26+
filePath := filepath.Join(tmpDir, faker.Word())
27+
err := fs.Touch(filePath)
28+
require.NoError(t, err)
29+
b, err := fs.IsFile(filePath)
30+
require.NoError(t, err)
31+
assert.True(t, b)
32+
})
33+
34+
t.Run("special file", func(t *testing.T) {
35+
if fsType == InMemoryFS {
36+
t.Skip("In-memory file system won't have hardware devices or special files")
37+
}
38+
39+
b, err := fs.IsFile("/dev/null")
40+
require.NoError(t, err)
41+
assert.True(t, b)
42+
43+
fifoPath := filepath.Join(tmpDir, faker.Word())
44+
require.NoError(t, err)
45+
defer func() { _ = fs.Rm(fifoPath) }()
46+
err = unix.Mkfifo(fifoPath, 0666)
47+
require.NoError(t, err)
48+
b, err = fs.IsFile(fifoPath)
49+
require.NoError(t, err)
50+
assert.True(t, b)
51+
err = fs.Rm(fifoPath)
52+
require.NoError(t, err)
53+
54+
socketPath := filepath.Join(tmpDir, faker.Word())
55+
require.NoError(t, err)
56+
defer func() { _ = fs.Rm(socketPath) }()
57+
l, err := net.Listen("unix", socketPath)
58+
require.NoError(t, err)
59+
defer func() { _ = l.Close() }()
60+
b, err = fs.IsFile(socketPath)
61+
require.NoError(t, err)
62+
assert.True(t, b)
63+
err = fs.Rm(socketPath)
64+
require.NoError(t, err)
65+
})
66+
})
67+
}
68+
}

‎utils/filesystem/lockfile_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"go.uber.org/goleak"
2020

2121
"github.com/ARM-software/golang-utils/utils/commonerrors"
22+
"github.com/ARM-software/golang-utils/utils/commonerrors/errortest"
2223
)
2324

2425
var (
@@ -361,18 +362,18 @@ func TestLockConcurrentSafeguard(t *testing.T) {
361362
// FIXME it was noticed that there could be some race conditions happening in the in-memory file system
362363
// see https://github.com/spf13/afero/issues/298
363364
if err1 != nil {
364-
require.Equal(t0, expectedError, err1)
365+
errortest.RequireError(t0, err1, expectedError)
365366
}
366367
if err2 != nil {
367-
require.Equal(t0, expectedError, err2)
368+
errortest.RequireError(t0, err2, expectedError)
368369
}
369370
} else {
370371
require.NotEqual(t0, err1, err2)
371372
if err1 == nil {
372-
require.Equal(t0, expectedError, err2)
373+
errortest.RequireError(t0, err2, expectedError)
373374
}
374375
if err2 == nil {
375-
require.Equal(t0, expectedError, err1)
376+
errortest.RequireError(t0, err1, expectedError)
376377
}
377378
}
378379
}

‎utils/parallelisation/contextual.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import (
88

99
// DetermineContextError determines what the context error is if any.
1010
func DetermineContextError(ctx context.Context) error {
11-
return commonerrors.ConvertContextError(ctx.Err())
11+
err := commonerrors.ConvertContextError(ctx.Err())
12+
if commonerrors.Any(err, nil) {
13+
return err
14+
}
15+
return commonerrors.WrapError(err, context.Cause(ctx), "")
1216
}
1317

1418
type ContextualFunc func(ctx context.Context) error

‎utils/parallelisation/contextual_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ package parallelisation
22

33
import (
44
"context"
5+
"errors"
56
"testing"
7+
"time"
68

9+
"github.com/stretchr/testify/assert"
710
"github.com/stretchr/testify/require"
811

912
"github.com/ARM-software/golang-utils/utils/commonerrors"
@@ -49,3 +52,37 @@ func TestForEach(t *testing.T) {
4952
require.NoError(t, ForEach(context.Background(), WithOptions(Workers(5), JoinErrors), WrapCancelToContextualFunc(cancelFunc), WrapCancelToContextualFunc(cancelFunc), WrapCancelToContextualFunc(cancelFunc)))
5053
})
5154
}
55+
56+
func TestDetermineContextError(t *testing.T) {
57+
t.Run("normal", func(t *testing.T) {
58+
require.NoError(t, DetermineContextError(context.Background()))
59+
})
60+
t.Run("cancellation", func(t *testing.T) {
61+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
62+
defer cancel()
63+
require.NoError(t, DetermineContextError(ctx))
64+
cancel()
65+
err := DetermineContextError(ctx)
66+
errortest.AssertError(t, err, commonerrors.ErrCancelled)
67+
})
68+
t.Run("cancellation with cause", func(t *testing.T) {
69+
cause := errors.New("a cause")
70+
ctx, cancel := context.WithCancelCause(context.Background())
71+
defer cancel(cause)
72+
require.NoError(t, DetermineContextError(ctx))
73+
cancel(cause)
74+
err := DetermineContextError(ctx)
75+
errortest.AssertError(t, err, commonerrors.ErrCancelled)
76+
errortest.AssertErrorDescription(t, err, cause.Error())
77+
})
78+
t.Run("cancellation with timeout cause", func(t *testing.T) {
79+
cause := errors.New("a cause")
80+
ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, cause)
81+
defer cancel()
82+
require.NoError(t, DetermineContextError(ctx))
83+
cancel()
84+
err := DetermineContextError(ctx)
85+
errortest.RequireError(t, err, commonerrors.ErrCancelled)
86+
assert.NotContains(t, err.Error(), cause.Error()) // the timeout did not take effect and a cancellation was performed instead so the cause is not passed through
87+
})
88+
}

0 commit comments

Comments
 (0)