Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: continue if app is not found in overview #2016

Merged
merged 2 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pkg/db/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -1089,14 +1089,15 @@ func (h *DBHandler) DBWriteAllApplications(ctx context.Context, transaction *sql
return fmt.Errorf("DBWriteAllApplications unable to get transaction timestamp: %w", err)
}
span.SetTag("query", insertQuery)
nextVersion := previousVersion + 1
_, err = transaction.Exec(
insertQuery,
previousVersion+1,
nextVersion,
*now,
jsonToInsert)

if err != nil {
return fmt.Errorf("could not insert all apps into DB. Error: %w\n", err)
return fmt.Errorf("could not insert all apps into DB with version=%d Error: %w\n", nextVersion, err)
}
return nil
}
Expand Down
14 changes: 13 additions & 1 deletion pkg/db/overview.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,19 @@ func (h *DBHandler) UpdateOverviewRelease(ctx context.Context, transaction *sql.
if release.Deleted {
return nil
}
return fmt.Errorf("could not find application '%s' in overview", release.App)
selectApp, err := h.DBSelectApp(ctx, transaction, release.App)
if err != nil {
return fmt.Errorf("could not find application '%s' in apps table: %w", release.App, err)
}
app = &api.Application{
Name: release.App,
Releases: []*api.Release{},
SourceRepoUrl: "", // TODO
Team: selectApp.Metadata.Team,
UndeploySummary: 0,
Warnings: []*api.Warning{},
}
latestOverview.Applications[release.App] = app
}
apiRelease := &api.Release{
PrNumber: extractPrNumber(release.Metadata.SourceMessage),
Expand Down
4 changes: 3 additions & 1 deletion pkg/db/transactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ func withTransactionAllOptions[T any](h *DBHandler, ctx context.Context, opts tr
maxRetries: opts.maxRetries - 1,
readonly: opts.readonly,
}, f)
} else {
logger.FromContext(ctx).Sugar().Warnf("%s transaction failed, will NOT retry error: %v", msg, e)
}
return nil, e
}
Expand Down Expand Up @@ -168,7 +170,7 @@ func IsRetryablePostgresError(err error) bool {
}
codeStr := string(pgErr.Code)
// for a list of all postgres error codes, see https://www.postgresql.org/docs/9.3/errcodes-appendix.html
return strings.HasPrefix(codeStr, "40")
return strings.HasPrefix(codeStr, "40") || strings.HasPrefix(codeStr, "23505")
}

func UnwrapUntilPostgresError(err error) *pq.Error {
Expand Down
13 changes: 12 additions & 1 deletion services/cd-service/pkg/repository/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import (
)

type CreateReleaseError struct {
response api.CreateReleaseResponse
response api.CreateReleaseResponse
innerError error
}

func (e *CreateReleaseError) Error() string {
Expand All @@ -44,11 +45,17 @@ func (e *CreateReleaseError) Is(target error) bool {
return proto.Equal(e.Response(), tgt.Response())
}

func (e *CreateReleaseError) Unwrap() error {
// Return the inner error.
return e.innerError
}

func GetCreateReleaseGeneralFailure(err error) *CreateReleaseError {
response := api.CreateReleaseResponseGeneralFailure{
Message: err.Error(),
}
return &CreateReleaseError{
innerError: err,
response: api.CreateReleaseResponse{
Response: &api.CreateReleaseResponse_GeneralFailure{
GeneralFailure: &response,
Expand All @@ -60,6 +67,7 @@ func GetCreateReleaseGeneralFailure(err error) *CreateReleaseError {
func GetCreateReleaseAlreadyExistsSame() *CreateReleaseError {
response := api.CreateReleaseResponseAlreadyExistsSame{}
return &CreateReleaseError{
innerError: nil,
response: api.CreateReleaseResponse{
Response: &api.CreateReleaseResponse_AlreadyExistsSame{
AlreadyExistsSame: &response,
Expand All @@ -74,6 +82,7 @@ func GetCreateReleaseAlreadyExistsDifferent(firstDifferingField api.DifferingFie
Diff: diff,
}
return &CreateReleaseError{
innerError: nil,
response: api.CreateReleaseResponse{
Response: &api.CreateReleaseResponse_AlreadyExistsDifferent{
AlreadyExistsDifferent: &response,
Expand All @@ -85,6 +94,7 @@ func GetCreateReleaseAlreadyExistsDifferent(firstDifferingField api.DifferingFie
func GetCreateReleaseTooOld() *CreateReleaseError {
response := api.CreateReleaseResponseTooOld{}
return &CreateReleaseError{
innerError: nil,
response: api.CreateReleaseResponse{
Response: &api.CreateReleaseResponse_TooOld{
TooOld: &response,
Expand All @@ -100,6 +110,7 @@ func GetCreateReleaseAppNameTooLong(appName string, regExp string, maxLen uint32
MaxLen: maxLen,
}
return &CreateReleaseError{
innerError: nil,
response: api.CreateReleaseResponse{
Response: &api.CreateReleaseResponse_TooLong{
TooLong: &response,
Expand Down
5 changes: 5 additions & 0 deletions services/cd-service/pkg/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ func (err *TransformerBatchApplyError) Is(target error) bool {
return errors.Is(err.TransformerError, tgt.TransformerError)
}

func (e *TransformerBatchApplyError) Unwrap() error {
// Return the inner error.
return e.TransformerError
}

func UnwrapUntilTransformerBatchApplyError(err error) *TransformerBatchApplyError {
for {
var applyErr *TransformerBatchApplyError
Expand Down
2 changes: 1 addition & 1 deletion services/cd-service/pkg/repository/transformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ func (c *CreateApplicationVersion) Transform(
allApps.Apps = append(allApps.Apps, c.Application)
err := state.DBHandler.DBWriteAllApplications(ctx, transaction, allApps.Version, allApps.Apps)
if err != nil {
return "", GetCreateReleaseGeneralFailure(fmt.Errorf("could not write all apps"))
return "", GetCreateReleaseGeneralFailure(fmt.Errorf("could not write all apps: %w", err))
}

//We need to check that this is not an app that has been previously deleted
Expand Down
29 changes: 29 additions & 0 deletions services/cd-service/pkg/repository/transformer_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/lib/pq"
"regexp"
"testing"
gotime "time"
Expand Down Expand Up @@ -3685,3 +3686,31 @@ func TestTimestampConsistency(t *testing.T) {
})
}
}

func TestIsCustomErrorRetryable(t *testing.T) {
tcs := []struct {
InputCode error
ExpectedRetryable bool
}{
{
InputCode: GetCreateReleaseGeneralFailure(fmt.Errorf("other2: %w", fmt.Errorf("foobar: %w", &pq.Error{Code: pq.ErrorCode("23505")}))),
ExpectedRetryable: true,
},
{
InputCode: GetCreateReleaseGeneralFailure(fmt.Errorf("could not write all apps: %w", &pq.Error{Code: pq.ErrorCode("23505")})),
ExpectedRetryable: true,
},
}
for _, tc := range tcs {
tc := tc
t.Run(fmt.Sprintf("endless_loop_check_%s", tc.InputCode.Error()), func(t *testing.T) {
t.Parallel()

actualRetryable := db.IsRetryablePostgresError(tc.InputCode)

if diff := cmp.Diff(tc.ExpectedRetryable, actualRetryable); diff != "" {
t.Fatalf("error mismatch (-want, +got):\n%s", diff)
}
})
}
}
Loading