fix: address review feedback on errcheck error handling

Signed-off-by: thev1ndu <itsthw9@gmail.com>
pull/32512/head
thev1ndu 2 months ago
parent 45966219b7
commit 0fcdff78f9

@ -71,7 +71,9 @@ var (
func TestTemplateIntegrationHappyPath(t *testing.T) { func TestTemplateIntegrationHappyPath(t *testing.T) {
// Rename file so it gets ignored by the linter // Rename file so it gets ignored by the linter
require.NoError(t, os.Rename(wrongTemplatePath, ignoredTemplatePath)) require.NoError(t, os.Rename(wrongTemplatePath, ignoredTemplatePath))
defer func() { _ = os.Rename(ignoredTemplatePath, wrongTemplatePath) }() defer func() {
assert.NoError(t, os.Rename(ignoredTemplatePath, wrongTemplatePath))
}()
linter := support.Linter{ChartDir: templateTestBasedir} linter := support.Linter{ChartDir: templateTestBasedir}
Templates(&linter, values, namespace, strict) Templates(&linter, values, namespace, strict)

@ -165,15 +165,20 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) {
// Sometimes users will provide a values.yaml for an argument where a chart is expected. One common occurrence // Sometimes users will provide a values.yaml for an argument where a chart is expected. One common occurrence
// of this is invoking `helm template values.yaml mychart` which would otherwise produce a confusing error // of this is invoking `helm template values.yaml mychart` which would otherwise produce a confusing error
// if we didn't check for this. // if we didn't check for this.
func EnsureArchive(name string, raw *os.File) error { func EnsureArchive(name string, raw *os.File) (err error) {
defer func() { defer func() {
_, _ = raw.Seek(0, 0) // reset read offset to allow archive loading to proceed. // Reset the read offset so archive loading can proceed. If the rewind
// fails and the check itself succeeded, surface the seek error rather
// than letting callers hit confusing follow-on errors.
if _, seekErr := raw.Seek(0, 0); seekErr != nil && err == nil {
err = fmt.Errorf("file '%s' cannot be reset: %w", name, seekErr)
}
}() }()
// Check the file format to give us a chance to provide the user with more actionable feedback. // Check the file format to give us a chance to provide the user with more actionable feedback.
buffer := make([]byte, 512) buffer := make([]byte, 512)
_, err := raw.Read(buffer) _, err = raw.Read(buffer)
if err != nil && err != io.EOF { if err != nil && !errors.Is(err, io.EOF) {
return fmt.Errorf("file '%s' cannot be read: %w", name, err) return fmt.Errorf("file '%s' cannot be read: %w", name, err)
} }

@ -72,7 +72,9 @@ var (
func TestTemplateIntegrationHappyPath(t *testing.T) { func TestTemplateIntegrationHappyPath(t *testing.T) {
// Rename file so it gets ignored by the linter // Rename file so it gets ignored by the linter
require.NoError(t, os.Rename(wrongTemplatePath, ignoredTemplatePath)) require.NoError(t, os.Rename(wrongTemplatePath, ignoredTemplatePath))
defer func() { _ = os.Rename(ignoredTemplatePath, wrongTemplatePath) }() defer func() {
assert.NoError(t, os.Rename(ignoredTemplatePath, wrongTemplatePath))
}()
linter := support.Linter{ChartDir: templateTestBasedir} linter := support.Linter{ChartDir: templateTestBasedir}
Templates( Templates(

@ -17,6 +17,7 @@ package repotest
import ( import (
"crypto/tls" "crypto/tls"
"errors"
"fmt" "fmt"
"net" "net"
"net/http" "net/http"
@ -215,7 +216,9 @@ func (srv *OCIServer) RunWithReturn(t *testing.T, opts ...OCIServerOpt) *OCIServ
} }
go func() { go func() {
_ = srv.ListenAndServe() if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
t.Errorf("OCI test registry server failed: %v", err)
}
}() }()
credentialsFile := filepath.Join(srv.Dir, "config.json") credentialsFile := filepath.Join(srv.Dir, "config.json")

@ -489,6 +489,8 @@ func (s *SQL) Create(key string, rel release.Releaser) error {
return fmt.Errorf("error beginning transaction: %w", err) return fmt.Errorf("error beginning transaction: %w", err)
} }
defer func() { _ = transaction.Rollback() }()
insertQuery, args, err := s.statementBuilder. insertQuery, args, err := s.statementBuilder.
Insert(sqlReleaseTableName). Insert(sqlReleaseTableName).
Columns( Columns(
@ -519,8 +521,6 @@ func (s *SQL) Create(key string, rel release.Releaser) error {
} }
if _, err := transaction.Exec(insertQuery, args...); err != nil { if _, err := transaction.Exec(insertQuery, args...); err != nil {
defer func() { _ = transaction.Rollback() }()
selectQuery, args, buildErr := s.statementBuilder. selectQuery, args, buildErr := s.statementBuilder.
Select(sqlReleaseTableKeyColumn). Select(sqlReleaseTableKeyColumn).
From(sqlReleaseTableName). From(sqlReleaseTableName).
@ -559,20 +559,17 @@ func (s *SQL) Create(key string, rel release.Releaser) error {
v, v,
).ToSql() ).ToSql()
if err != nil { if err != nil {
defer func() { _ = transaction.Rollback() }()
s.Logger().Debug("failed to build insert query", slog.Any("error", err)) s.Logger().Debug("failed to build insert query", slog.Any("error", err))
return err return err
} }
if _, err := transaction.Exec(insertLabelsQuery, args...); err != nil { if _, err := transaction.Exec(insertLabelsQuery, args...); err != nil {
defer func() { _ = transaction.Rollback() }()
s.Logger().Debug("failed to write Labels", slog.Any("error", err)) s.Logger().Debug("failed to write Labels", slog.Any("error", err))
return err return err
} }
} }
defer func() { _ = transaction.Commit() }()
return nil return transaction.Commit()
} }
// Update updates a release. // Update updates a release.
@ -624,6 +621,9 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
s.Logger().Debug("failed to start SQL transaction", slog.Any("error", err)) s.Logger().Debug("failed to start SQL transaction", slog.Any("error", err))
return nil, fmt.Errorf("error beginning transaction: %w", err) return nil, fmt.Errorf("error beginning transaction: %w", err)
} }
// Roll back the transaction on any early return. After a successful
// Commit this is a no-op that returns sql.ErrTxDone, which we ignore.
defer func() { _ = transaction.Rollback() }()
selectQuery, args, err := s.statementBuilder. selectQuery, args, err := s.statementBuilder.
Select(sqlReleaseTableBodyColumn). Select(sqlReleaseTableBodyColumn).
@ -646,10 +646,8 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
release, err := decodeRelease(record.Body) release, err := decodeRelease(record.Body)
if err != nil { if err != nil {
s.Logger().Debug("failed to decode release", slog.String("key", key), slog.Any("error", err)) s.Logger().Debug("failed to decode release", slog.String("key", key), slog.Any("error", err))
_ = transaction.Rollback()
return nil, err return nil, err
} }
defer func() { _ = transaction.Commit() }()
deleteQuery, args, err := s.statementBuilder. deleteQuery, args, err := s.statementBuilder.
Delete(sqlReleaseTableName). Delete(sqlReleaseTableName).
@ -664,7 +662,7 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
_, err = transaction.Exec(deleteQuery, args...) _, err = transaction.Exec(deleteQuery, args...)
if err != nil { if err != nil {
s.Logger().Debug("failed perform delete query", slog.Any("error", err)) s.Logger().Debug("failed perform delete query", slog.Any("error", err))
return release, err return nil, err
} }
if release.Labels, err = s.getReleaseCustomLabels(key, s.namespace); err != nil { if release.Labels, err = s.getReleaseCustomLabels(key, s.namespace); err != nil {
@ -685,8 +683,17 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
s.Logger().Debug("failed to build delete Labels query", slog.Any("error", err)) s.Logger().Debug("failed to build delete Labels query", slog.Any("error", err))
return nil, err return nil, err
} }
_, err = transaction.Exec(deleteCustomLabelsQuery, args...) if _, err = transaction.Exec(deleteCustomLabelsQuery, args...); err != nil {
return release, err s.Logger().Debug("failed to perform delete Labels query", slog.Any("error", err))
return nil, err
}
if err := transaction.Commit(); err != nil {
s.Logger().Debug("failed to commit delete transaction", slog.Any("error", err))
return nil, err
}
return release, nil
} }
// Get release custom labels from database // Get release custom labels from database

Loading…
Cancel
Save