Skip to content

Commit

Permalink
fix(gcs): Wrap errors using %w to make retrying work (#9280)
Browse files Browse the repository at this point in the history
We have a function `isTransientGCSErr()` here which attempts to
`Unwrap()` errors which we have wrapped using `fmt.Errorf()`. This will
only work if the `%w` format string has been used, otherwise no
`Unwrap()` method is created.

This should enable retrying on transient GCS upload errors, and avoid
failures like

```
executor error: upload /tmp/argo/outputs/logs/main.log: writer close:
googleapi: Error 503: We encountered an internal error. Please try
again., backendError
```

Which we've been seeing.

Signed-off-by: Iain Lane <[email protected]>
  • Loading branch information
iainlane committed Aug 3, 2022
1 parent 083f3a2 commit 06b0a8c
Showing 1 changed file with 13 additions and 13 deletions.
26 changes: 13 additions & 13 deletions workflow/artifacts/gcs/gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@ func newGCSClientWithCredential(serviceAccountJSON string) (*storage.Client, err
ctx := context.Background()
creds, err := google.CredentialsFromJSON(ctx, []byte(serviceAccountJSON), storage.ScopeReadWrite)
if err != nil {
return nil, fmt.Errorf("GCS client CredentialsFromJSON: %v", err)
return nil, fmt.Errorf("GCS client CredentialsFromJSON: %w", err)
}
client, err := storage.NewClient(ctx, option.WithCredentials(creds))
if err != nil {
return nil, fmt.Errorf("GCS storage.NewClient with credential: %v", err)
return nil, fmt.Errorf("GCS storage.NewClient with credential: %w", err)
}
return client, nil
}
Expand All @@ -92,7 +92,7 @@ func newGCSClientDefault() (*storage.Client, error) {
ctx := context.Background()
client, err := storage.NewClient(ctx)
if err != nil {
return nil, fmt.Errorf("GCS storage.NewClient: %v", err)
return nil, fmt.Errorf("GCS storage.NewClient: %w", err)
}
return client, nil
}
Expand Down Expand Up @@ -149,7 +149,7 @@ func downloadObject(client *storage.Client, bucket, key, objName, path string) e
objectDir, _ := filepath.Split(localPath)
if objectDir != "" {
if err := os.MkdirAll(objectDir, 0o700); err != nil {
return fmt.Errorf("mkdir %s: %v", objectDir, err)
return fmt.Errorf("mkdir %s: %w", objectDir, err)
}
}
ctx := context.Background()
Expand All @@ -158,12 +158,12 @@ func downloadObject(client *storage.Client, bucket, key, objName, path string) e
if err == storage.ErrObjectNotExist {
return errors.New(errors.CodeNotFound, err.Error())
}
return fmt.Errorf("new bucket reader: %v", err)
return fmt.Errorf("new bucket reader: %w", err)
}
defer rc.Close()
out, err := os.Create(localPath)
if err != nil {
return fmt.Errorf("os create %s: %v", localPath, err)
return fmt.Errorf("os create %s: %w", localPath, err)
}
defer func() {
if err := out.Close(); err != nil {
Expand All @@ -172,7 +172,7 @@ func downloadObject(client *storage.Client, bucket, key, objName, path string) e
}()
_, err = io.Copy(out, rc)
if err != nil {
return fmt.Errorf("io copy: %v", err)
return fmt.Errorf("io copy: %w", err)
}
return nil
}
Expand Down Expand Up @@ -251,7 +251,7 @@ func listFileRelPaths(path string, relPath string) ([]string, error) {
func uploadObjects(client *storage.Client, bucket, key, path string) error {
isDir, err := file.IsDirectory(path)
if err != nil {
return fmt.Errorf("test if %s is a dir: %v", path, err)
return fmt.Errorf("test if %s is a dir: %w", path, err)
}
if isDir {
dirName := filepath.Clean(path) + string(os.PathSeparator)
Expand All @@ -268,7 +268,7 @@ func uploadObjects(client *storage.Client, bucket, key, path string) error {

err = uploadObject(client, bucket, fullKey, dirName+relPath)
if err != nil {
return fmt.Errorf("upload %s: %v", dirName+relPath, err)
return fmt.Errorf("upload %s: %w", dirName+relPath, err)
}
}
} else {
Expand All @@ -278,7 +278,7 @@ func uploadObjects(client *storage.Client, bucket, key, path string) error {
}
err = uploadObject(client, bucket, objectKey, path)
if err != nil {
return fmt.Errorf("upload %s: %v", path, err)
return fmt.Errorf("upload %s: %w", path, err)
}
}
return nil
Expand All @@ -288,7 +288,7 @@ func uploadObjects(client *storage.Client, bucket, key, path string) error {
func uploadObject(client *storage.Client, bucket, key, localPath string) error {
f, err := os.Open(filepath.Clean(localPath))
if err != nil {
return fmt.Errorf("os open: %v", err)
return fmt.Errorf("os open: %w", err)
}
defer func() {
if err := f.Close(); err != nil {
Expand All @@ -298,10 +298,10 @@ func uploadObject(client *storage.Client, bucket, key, localPath string) error {
ctx := context.Background()
wc := client.Bucket(bucket).Object(key).NewWriter(ctx)
if _, err = io.Copy(wc, f); err != nil {
return fmt.Errorf("io copy: %v", err)
return fmt.Errorf("io copy: %w", err)
}
if err := wc.Close(); err != nil {
return fmt.Errorf("writer close: %v", err)
return fmt.Errorf("writer close: %w", err)
}
return nil
}
Expand Down

0 comments on commit 06b0a8c

Please sign in to comment.