From 42391bbecd908d73bfcb6d0d95ce03290ec20655 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 17 Oct 2024 13:50:15 -0700 Subject: [PATCH 1/5] Handle more errors --- internal/locale/locales/en-us.yaml | 2 + internal/runners/pull/rationalize.go | 2 +- internal/runners/push/rationalize.go | 2 +- .../api/buildplanner/request/createproject.go | 9 ++- .../api/buildplanner/request/mergecommit.go | 12 +++ .../api/buildplanner/request/revertcommit.go | 17 +++- .../api/buildplanner/request/stagecommit.go | 5 +- .../api/buildplanner/response/commit.go | 8 +- .../api/buildplanner/response/commiterror.go | 80 ++++++++++++++++--- .../api/buildplanner/response/shared.go | 6 +- pkg/platform/api/buildplanner/types/errors.go | 3 +- 11 files changed, 124 insertions(+), 22 deletions(-) diff --git a/internal/locale/locales/en-us.yaml b/internal/locale/locales/en-us.yaml index bf6e64f9db..c291b94cf2 100644 --- a/internal/locale/locales/en-us.yaml +++ b/internal/locale/locales/en-us.yaml @@ -1031,6 +1031,8 @@ buildplan_err_cropped: other: "Could not plan build. Platform responded with:\n{{.V0}}\n{{.V1}}" err_buildplanner_head_on_branch_moved: other: The branch you're trying to update has changed remotely. Please run '[ACTIONABLE]state pull[/RESET]'. +err_buildplanner_forbidden: + other: "Operation forbidden: {{.V0}}. Received message: {{.V1}}" transient_solver_tip: other: You may want to retry this command if the failure was related to a resourcing or networking issue. warning_git_project_mismatch: diff --git a/internal/runners/pull/rationalize.go b/internal/runners/pull/rationalize.go index 6bcdea0a11..26cb3dabcd 100644 --- a/internal/runners/pull/rationalize.go +++ b/internal/runners/pull/rationalize.go @@ -24,7 +24,7 @@ func rationalizeError(err *error) { case errors.As(*err, &mergeCommitErr): switch mergeCommitErr.Type { // Custom target does not have a compatible history - case types.NoCommonBaseFoundType: + case types.NoCommonBaseFoundErrorType: *err = errs.WrapUserFacing(*err, locale.Tl("err_pull_no_common_base", "Could not merge. No common base found between local and remote commits", diff --git a/internal/runners/push/rationalize.go b/internal/runners/push/rationalize.go index 90af8e6774..ab8e8b25ca 100644 --- a/internal/runners/push/rationalize.go +++ b/internal/runners/push/rationalize.go @@ -82,7 +82,7 @@ func rationalizeError(err *error) { errs.SetTips(locale.T("err_tip_push_outdated"))) // Custom target does not have a compatible history - case types.NoCommonBaseFoundType: + case types.NoCommonBaseFoundErrorType: *err = errs.WrapUserFacing(*err, locale.T("err_push_target_invalid_history"), errs.SetInput()) diff --git a/pkg/platform/api/buildplanner/request/createproject.go b/pkg/platform/api/buildplanner/request/createproject.go index 8f3c52aca0..92c086e49c 100644 --- a/pkg/platform/api/buildplanner/request/createproject.go +++ b/pkg/platform/api/buildplanner/request/createproject.go @@ -37,11 +37,18 @@ mutation ($organization: String!, $project: String!, $private: Boolean!, $expr: ... on ParseError { __typename message - path + subErrors { + message + buildExprPath + } } ... on ValidationError { __typename message + subErrors { + message + buildExprPath + } } ... on Forbidden { __typename diff --git a/pkg/platform/api/buildplanner/request/mergecommit.go b/pkg/platform/api/buildplanner/request/mergecommit.go index b1868d5f2d..ad34cf57c0 100644 --- a/pkg/platform/api/buildplanner/request/mergecommit.go +++ b/pkg/platform/api/buildplanner/request/mergecommit.go @@ -48,10 +48,18 @@ mutation ($organization: String!, $project: String!, $targetRef: String!, $other ... on ParseError { __typename message + subErrors { + message + buildExprPath + } } ... on ValidationError { __typename message + subErrors { + message + buildExprPath + } } ... on Forbidden { __typename @@ -68,6 +76,10 @@ mutation ($organization: String!, $project: String!, $targetRef: String!, $other message commitId } + ... on InvalidInput { + __typename + message + } } } ` diff --git a/pkg/platform/api/buildplanner/request/revertcommit.go b/pkg/platform/api/buildplanner/request/revertcommit.go index 1a2e4fbe5a..25844ac787 100644 --- a/pkg/platform/api/buildplanner/request/revertcommit.go +++ b/pkg/platform/api/buildplanner/request/revertcommit.go @@ -42,6 +42,10 @@ mutation ($organization: String!, $project: String!, $commitId: String!, $target targetCommitId conflictPaths } + ... on CommitNotInTargetHistory { + __typename + message + } ... on CommitHasNoParent { __typename message @@ -54,11 +58,18 @@ mutation ($organization: String!, $project: String!, $commitId: String!, $target ... on ParseError { __typename message - path + subErrors { + message + buildExprPath + } } ... on ValidationError { __typename message + subErrors { + message + buildExprPath + } } ... on Forbidden { __typename @@ -72,6 +83,10 @@ mutation ($organization: String!, $project: String!, $commitId: String!, $target message commitId } + ... on InvalidInput { + __typename + message + } } }` } diff --git a/pkg/platform/api/buildplanner/request/stagecommit.go b/pkg/platform/api/buildplanner/request/stagecommit.go index 16b5f9d30c..42725f51ad 100644 --- a/pkg/platform/api/buildplanner/request/stagecommit.go +++ b/pkg/platform/api/buildplanner/request/stagecommit.go @@ -212,7 +212,10 @@ mutation ($organization: String!, $project: String!, $parentCommit: ID!, $descri ... on ParseError { __typename message - path + subErrors { + message + buildExprPath + } } ... on Forbidden { __typename diff --git a/pkg/platform/api/buildplanner/response/commit.go b/pkg/platform/api/buildplanner/response/commit.go index 127613883b..732a708334 100644 --- a/pkg/platform/api/buildplanner/response/commit.go +++ b/pkg/platform/api/buildplanner/response/commit.go @@ -54,7 +54,7 @@ func (c *ProjectCommitResponse) PostProcess() error { func ProcessBuildError(build *BuildResponse, fallbackMessage string) error { logging.Debug("ProcessBuildError: build.Type=%s", build.Type) if build.Type == types.PlanningErrorType { - return processPlanningError(build.Message, build.SubErrors) + return processPlanningError(build.Message, build.Error.SubErrors) } else if build.Error == nil { return errs.New(fallbackMessage) } @@ -133,9 +133,9 @@ type TargetNotFound struct { PossibleTargets []string `json:"possibleTargets"` } -type ValidationError struct { - SubErrors []*BuildExprError `json:"subErrors"` -} +// ValidationError represents a validation error that occurred during planning. +// Contains message and subErrors which is handled separately +type ValidationError struct{} // Commit contains the build and any errors. type Commit struct { diff --git a/pkg/platform/api/buildplanner/response/commiterror.go b/pkg/platform/api/buildplanner/response/commiterror.go index e5efef0793..1a0f53192a 100644 --- a/pkg/platform/api/buildplanner/response/commiterror.go +++ b/pkg/platform/api/buildplanner/response/commiterror.go @@ -27,9 +27,19 @@ func ProcessCommitError(commit *Commit, fallbackMessage string) error { locale.NewInputError("err_buildplanner_commit_not_found", "Could not find commit. Received message: {{.V0}}", commit.Message), } case types.ParseErrorType: + var subErrorMessages []string + for _, e := range commit.SubErrors { + subErrorMessages = append(subErrorMessages, e.Message) + } + if len(subErrorMessages) > 0 { + return &CommitError{ + commit.Type, commit.Message, + locale.NewInputError("err_buildplanner_commit_parse_error_sub_messages", "The platform failed to parse the build expression. Received message: {{.V0}}, with sub errors: {{.V1}}", commit.Message, strings.Join(subErrorMessages, ", ")), + } + } return &CommitError{ commit.Type, commit.Message, - locale.NewInputError("err_buildplanner_parse_error", "The platform failed to parse the build expression. Received message: {{.V0}}. Path: {{.V1}}", commit.Message, commit.ParseError.Path), + locale.NewInputError("err_buildplanner_commit_parse_error", "The platform failed to parse the build expression. Received message: {{.V0}}", commit.Message), } case types.ValidationErrorType: var subErrorMessages []string @@ -39,17 +49,17 @@ func ProcessCommitError(commit *Commit, fallbackMessage string) error { if len(subErrorMessages) > 0 { return &CommitError{ commit.Type, commit.Message, - locale.NewInputError("err_buildplanner_validation_error_sub_messages", "The platform encountered a validation error. Received message: {{.V0}}, with sub errors: {{.V1}}", commit.Message, strings.Join(subErrorMessages, ", ")), + locale.NewInputError("err_buildplanner_commit_validation_error_sub_messages", "The platform encountered a validation error. Received message: {{.V0}}, with sub errors: {{.V1}}", commit.Message, strings.Join(subErrorMessages, ", ")), } } return &CommitError{ commit.Type, commit.Message, - locale.NewInputError("err_buildplanner_validation_error", "The platform encountered a validation error. Received message: {{.V0}}", commit.Message), + locale.NewInputError("err_buildplanner_commit_validation_error", "The platform encountered a validation error. Received message: {{.V0}}", commit.Message), } case types.ForbiddenErrorType: return &CommitError{ commit.Type, commit.Message, - locale.NewInputError("err_buildplanner_forbidden", "Operation forbidden: {{.V0}}. Received message: {{.V1}}", commit.Operation, commit.Message), + locale.NewInputError("err_buildplanner_forbidden", commit.Operation, commit.Message), } case types.HeadOnBranchMovedErrorType: return errs.Wrap(&CommitError{ @@ -69,29 +79,79 @@ func ProcessCommitError(commit *Commit, fallbackMessage string) error { type RevertCommitError struct { Type string Message string + *locale.LocalizedError } func (m *RevertCommitError) Error() string { return m.Message } func ProcessRevertCommitError(rcErr *revertedCommit, fallbackMessage string) error { - if rcErr.Type != "" { - return &RevertCommitError{rcErr.Type, rcErr.Message} + if rcErr.Error == nil { + return errs.New(fallbackMessage) + } + + switch rcErr.Type { + case types.RevertConflictErrorType: + return &RevertCommitError{ + rcErr.Type, rcErr.Message, + locale.NewInputError("err_buildplanner_revert_conflict", "The revert operation could not be completed due to a conflict. Received message: {{.V0}}", rcErr.Message), + } + case types.CommitNotInTargetHistoryErrorType: + return &RevertCommitError{ + rcErr.Type, rcErr.Message, + locale.NewInputError("err_buildplanner_commit_revert_not_in_target_history", "The commit to revert is not in the target history. Received message: {{.V0}}", rcErr.Message), + } + case types.ComitHasNoParentErrorType: + return &RevertCommitError{ + rcErr.Type, rcErr.Message, + locale.NewInputError("err_buildplanner_commit_revert_has_no_parent", "The commit to revert has no parent. Received message: {{.V0}}", rcErr.Message), + } + case types.InvalidInputErrorType: + return &RevertCommitError{ + rcErr.Type, rcErr.Message, + locale.NewInputError("err_buildplanner_revert_invalid_input", "The input to the revert operation was invalid. Received message: {{.V0}}", rcErr.Message), + } + default: + return errs.New(fallbackMessage) } - return errs.New(fallbackMessage) } type MergedCommitError struct { Type string Message string + *locale.LocalizedError } func (m *MergedCommitError) Error() string { return m.Message } func ProcessMergedCommitError(mcErr *mergedCommit, fallbackMessage string) error { - if mcErr.Type != "" { - return &MergedCommitError{mcErr.Type, mcErr.Message} + if mcErr.Error == nil { + return errs.New(fallbackMessage) + } + + switch mcErr.Type { + case types.MergeConflictErrorType: + return &MergedCommitError{ + mcErr.Type, mcErr.Message, + locale.NewInputError("err_buildplanner_merge_conflict", "The platform encountered a merge conflict. Received message: {{.V0}}", mcErr.Message), + } + case types.FastForwardErrorType: + return &MergedCommitError{ + mcErr.Type, mcErr.Message, + locale.NewInputError("err_buildplanner_merge_fast_forward_error", "The platform could not merge with the Fast Forward strategy. Received message: {{.V0}}", mcErr.Message), + } + case types.NoCommonBaseFoundErrorType: + return &MergedCommitError{ + mcErr.Type, mcErr.Message, + locale.NewInputError("err_buildplanner_merge_no_common_base_found", "The platform could not find a common base for the merge. Received message: {{.V0}}", mcErr.Message), + } + case types.InvalidInputErrorType: + return &MergedCommitError{ + mcErr.Type, mcErr.Message, + locale.NewInputError("err_buildplanner_merge_invalid_input", "The input to the merge commit mutation was invalid. Received message: {{.V0}}", mcErr.Message), + } + default: + return errs.New(fallbackMessage) } - return errs.New(fallbackMessage) } // HeadOnBranchMovedError represents an error that occurred because the head on diff --git a/pkg/platform/api/buildplanner/response/shared.go b/pkg/platform/api/buildplanner/response/shared.go index 627bfcb949..ed958b6a61 100644 --- a/pkg/platform/api/buildplanner/response/shared.go +++ b/pkg/platform/api/buildplanner/response/shared.go @@ -85,7 +85,7 @@ func IsErrorResponse(errorType string) bool { errorType == types.PlanningErrorType || errorType == types.MergeConflictType || errorType == types.FastForwardErrorType || - errorType == types.NoCommonBaseFoundType || + errorType == types.NoCommonBaseFoundErrorType || errorType == types.ValidationErrorType || errorType == types.MergeConflictErrorType || errorType == types.RevertConflictErrorType || @@ -101,6 +101,7 @@ type NotFoundError struct { } // ParseError is an error that occurred while parsing the build expression. +// SubErrors are handled separately type ParseError struct { Path string `json:"path"` } @@ -111,7 +112,8 @@ type ForbiddenError struct { // Error contains an error message. type Error struct { - Message string `json:"message"` + Message string `json:"message"` + SubErrors []*BuildExprError `json:"subErrors"` } type TargetNotFoundError struct { diff --git a/pkg/platform/api/buildplanner/types/errors.go b/pkg/platform/api/buildplanner/types/errors.go index e3f234d30a..d34dae9ea9 100644 --- a/pkg/platform/api/buildplanner/types/errors.go +++ b/pkg/platform/api/buildplanner/types/errors.go @@ -14,11 +14,12 @@ const ( PlanningErrorType = "PlanningError" MergeConflictType = "MergeConflict" FastForwardErrorType = "FastForwardError" - NoCommonBaseFoundType = "NoCommonBaseFound" + NoCommonBaseFoundErrorType = "NoCommonBaseFound" ValidationErrorType = "ValidationError" MergeConflictErrorType = "MergeConflict" RevertConflictErrorType = "RevertConflict" CommitNotInTargetHistoryErrorType = "CommitNotInTargetHistory" ComitHasNoParentErrorType = "CommitHasNoParent" TargetNotFoundErrorType = "TargetNotFound" + InvalidInputErrorType = "InvalidInput" ) From b74438bddfb3f3838669163755e2aeb9b8f46395 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 17 Oct 2024 15:20:37 -0700 Subject: [PATCH 2/5] Increase timeout --- test/integration/exec_int_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/integration/exec_int_test.go b/test/integration/exec_int_test.go index 1bc3009ac5..54dfabd352 100644 --- a/test/integration/exec_int_test.go +++ b/test/integration/exec_int_test.go @@ -8,7 +8,6 @@ import ( "runtime" "strings" "testing" - "time" "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/environment" @@ -16,7 +15,6 @@ import ( "github.com/ActiveState/cli/internal/testhelpers/e2e" "github.com/ActiveState/cli/internal/testhelpers/suite" "github.com/ActiveState/cli/internal/testhelpers/tagsuite" - "github.com/ActiveState/termtest" ) type ExecIntegrationTestSuite struct { @@ -176,7 +174,7 @@ func (suite *ExecIntegrationTestSuite) TestExeBatArguments() { inputs := []string{"aa", "hello world", "&whoami", "imnot|apipe", "%NotAppData%", "^NotEscaped", "(NotAGroup)"} outputs := `"` + strings.Join(inputs, `" "`) + `"` cp = ts.SpawnWithOpts(e2e.OptArgs(append([]string{"exec", reportBat, "--"}, inputs...)...)) - cp.Expect(outputs, termtest.OptExpectTimeout(5*time.Second)) + cp.Expect(outputs, e2e.RuntimeSourcingTimeoutOpt) cp.ExpectExitCode(0) } From 423eb8e93380a513f437df0e12df009a016fcfa0 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 17 Oct 2024 15:21:57 -0700 Subject: [PATCH 3/5] Revert "Increase timeout" This reverts commit b74438bddfb3f3838669163755e2aeb9b8f46395. --- test/integration/exec_int_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/integration/exec_int_test.go b/test/integration/exec_int_test.go index 54dfabd352..1bc3009ac5 100644 --- a/test/integration/exec_int_test.go +++ b/test/integration/exec_int_test.go @@ -8,6 +8,7 @@ import ( "runtime" "strings" "testing" + "time" "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/environment" @@ -15,6 +16,7 @@ import ( "github.com/ActiveState/cli/internal/testhelpers/e2e" "github.com/ActiveState/cli/internal/testhelpers/suite" "github.com/ActiveState/cli/internal/testhelpers/tagsuite" + "github.com/ActiveState/termtest" ) type ExecIntegrationTestSuite struct { @@ -174,7 +176,7 @@ func (suite *ExecIntegrationTestSuite) TestExeBatArguments() { inputs := []string{"aa", "hello world", "&whoami", "imnot|apipe", "%NotAppData%", "^NotEscaped", "(NotAGroup)"} outputs := `"` + strings.Join(inputs, `" "`) + `"` cp = ts.SpawnWithOpts(e2e.OptArgs(append([]string{"exec", reportBat, "--"}, inputs...)...)) - cp.Expect(outputs, e2e.RuntimeSourcingTimeoutOpt) + cp.Expect(outputs, termtest.OptExpectTimeout(5*time.Second)) cp.ExpectExitCode(0) } From e40149018cc6ddded29a94f7378769357cba8de2 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 22 Oct 2024 10:03:36 -0700 Subject: [PATCH 4/5] Move atTime out of raw --- internal/runbits/runtime/rationalize.go | 2 ++ pkg/buildscript/buildscript.go | 6 ++++-- pkg/buildscript/marshal.go | 4 ++-- pkg/buildscript/marshal_buildexpression.go | 2 +- pkg/buildscript/raw.go | 3 --- pkg/buildscript/unmarshal.go | 7 ++++--- pkg/buildscript/unmarshal_buildexpression.go | 7 ------- pkg/platform/model/buildplanner/buildscript.go | 2 +- 8 files changed, 14 insertions(+), 19 deletions(-) diff --git a/internal/runbits/runtime/rationalize.go b/internal/runbits/runtime/rationalize.go index 3feac4ac8d..60426f9850 100644 --- a/internal/runbits/runtime/rationalize.go +++ b/internal/runbits/runtime/rationalize.go @@ -122,6 +122,8 @@ func RationalizeSolveError(proj *project.Project, auth *auth.Auth, rerr *error) ) // We communicate buildplanner errors verbatim as the intend is that these are curated by the buildplanner + // TODO: Maybe we should just forward any error message here? + // This could simplify the graphQL error handling. case errors.As(*rerr, &buildPlannerErr): *rerr = errs.WrapUserFacing(*rerr, buildPlannerErr.LocaleError(), diff --git a/pkg/buildscript/buildscript.go b/pkg/buildscript/buildscript.go index fcaf7981c7..3298f5a290 100644 --- a/pkg/buildscript/buildscript.go +++ b/pkg/buildscript/buildscript.go @@ -16,6 +16,8 @@ import ( // methods that are easy to understand and work with. type BuildScript struct { raw *rawBuildScript + + atTime *time.Time // set after initial read } func init() { @@ -42,11 +44,11 @@ func New() *BuildScript { } func (b *BuildScript) AtTime() *time.Time { - return b.raw.AtTime + return b.atTime } func (b *BuildScript) SetAtTime(t time.Time) { - b.raw.AtTime = &t + b.atTime = &t } func (b *BuildScript) Equals(other *BuildScript) (bool, error) { diff --git a/pkg/buildscript/marshal.go b/pkg/buildscript/marshal.go index c647616f47..173d7b7c34 100644 --- a/pkg/buildscript/marshal.go +++ b/pkg/buildscript/marshal.go @@ -28,9 +28,9 @@ const ( func (b *BuildScript) Marshal() ([]byte, error) { buf := strings.Builder{} - if b.raw.AtTime != nil { + if b.atTime != nil { buf.WriteString(assignmentString( - &assignment{atTimeKey, newString(b.raw.AtTime.Format(strfmt.RFC3339Millis))})) + &assignment{atTimeKey, newString(b.atTime.Format(strfmt.RFC3339Millis))})) buf.WriteString("\n") } diff --git a/pkg/buildscript/marshal_buildexpression.go b/pkg/buildscript/marshal_buildexpression.go index cb8a152803..15a6f62ef7 100644 --- a/pkg/buildscript/marshal_buildexpression.go +++ b/pkg/buildscript/marshal_buildexpression.go @@ -41,7 +41,7 @@ func (b *BuildScript) MarshalBuildExpression() ([]byte, error) { if err != nil { return nil, errs.Wrap(err, "Invalid timestamp: %s", strValue(value)) } - b.raw.AtTime = ptr.To(time.Time(atTime)) + b.atTime = ptr.To(time.Time(atTime)) continue // do not include this custom assignment in the let block case mainKey: key = inKey // rename diff --git a/pkg/buildscript/raw.go b/pkg/buildscript/raw.go index 8a2dbc3149..51f6ede04f 100644 --- a/pkg/buildscript/raw.go +++ b/pkg/buildscript/raw.go @@ -3,7 +3,6 @@ package buildscript import ( "strconv" "strings" - "time" "github.com/ActiveState/cli/internal/rtutils/ptr" "github.com/brunoga/deep" @@ -12,8 +11,6 @@ import ( // Tagged fields will be filled in by Participle. type rawBuildScript struct { Assignments []*assignment `parser:"@@+"` - - AtTime *time.Time // set after initial read } // clone is meant to facilitate making modifications to functions at marshal time. The idea is that these modifications diff --git a/pkg/buildscript/unmarshal.go b/pkg/buildscript/unmarshal.go index ffbab27f9e..4ae616089f 100644 --- a/pkg/buildscript/unmarshal.go +++ b/pkg/buildscript/unmarshal.go @@ -32,6 +32,7 @@ func Unmarshal(data []byte) (*BuildScript, error) { } // Extract 'at_time' value from the list of assignments, if it exists. + var atTime *time.Time for i, assignment := range raw.Assignments { key := assignment.Key value := assignment.Value @@ -42,11 +43,11 @@ func Unmarshal(data []byte) (*BuildScript, error) { if value.Str == nil { break } - atTime, err := strfmt.ParseDateTime(strValue(value)) + parsedAtTime, err := strfmt.ParseDateTime(strValue(value)) if err != nil { return nil, errs.Wrap(err, "Invalid timestamp: %s", strValue(value)) } - raw.AtTime = ptr.To(time.Time(atTime)) + atTime = ptr.To(time.Time(parsedAtTime)) break } @@ -60,5 +61,5 @@ func Unmarshal(data []byte) (*BuildScript, error) { seen[assignment.Key] = true } - return &BuildScript{raw}, nil + return &BuildScript{raw, atTime}, nil } diff --git a/pkg/buildscript/unmarshal_buildexpression.go b/pkg/buildscript/unmarshal_buildexpression.go index 77af730583..f4639aac5f 100644 --- a/pkg/buildscript/unmarshal_buildexpression.go +++ b/pkg/buildscript/unmarshal_buildexpression.go @@ -5,9 +5,7 @@ import ( "sort" "strconv" "strings" - "time" - "github.com/go-openapi/strfmt" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -70,13 +68,8 @@ func (b *BuildScript) UnmarshalBuildExpression(data []byte) error { // Extract the 'at_time' from the solve node, if it exists, and change its value to be a // reference to "$at_time", which is how we want to show it in AScript format. if atTimeNode, err := b.getSolveAtTimeValue(); err == nil && atTimeNode.Str != nil && !strings.HasPrefix(strValue(atTimeNode), `$`) { - atTime, err := strfmt.ParseDateTime(strValue(atTimeNode)) - if err != nil { - return errs.Wrap(err, "Invalid timestamp: %s", strValue(atTimeNode)) - } atTimeNode.Str = nil atTimeNode.Ident = ptr.To("at_time") - b.raw.AtTime = ptr.To(time.Time(atTime)) } else if err != nil { return errs.Wrap(err, "Could not get at_time node") } diff --git a/pkg/platform/model/buildplanner/buildscript.go b/pkg/platform/model/buildplanner/buildscript.go index 28bd980ffc..52f038ee5f 100644 --- a/pkg/platform/model/buildplanner/buildscript.go +++ b/pkg/platform/model/buildplanner/buildscript.go @@ -52,10 +52,10 @@ func (b *BuildPlanner) GetBuildScript(commitID string) (*buildscript.BuildScript } script := buildscript.New() - script.SetAtTime(time.Time(resp.Commit.AtTime)) if err := script.UnmarshalBuildExpression(resp.Commit.Expression); err != nil { return nil, errs.Wrap(err, "failed to parse build expression") } + script.SetAtTime(time.Time(resp.Commit.AtTime)) return script, nil } From 1e2790e8fcdc21d3debcf48ce410e33d63803558 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 22 Oct 2024 10:06:24 -0700 Subject: [PATCH 5/5] Revert "Move atTime out of raw" This reverts commit e40149018cc6ddded29a94f7378769357cba8de2. --- internal/runbits/runtime/rationalize.go | 2 -- pkg/buildscript/buildscript.go | 6 ++---- pkg/buildscript/marshal.go | 4 ++-- pkg/buildscript/marshal_buildexpression.go | 2 +- pkg/buildscript/raw.go | 3 +++ pkg/buildscript/unmarshal.go | 7 +++---- pkg/buildscript/unmarshal_buildexpression.go | 7 +++++++ pkg/platform/model/buildplanner/buildscript.go | 2 +- 8 files changed, 19 insertions(+), 14 deletions(-) diff --git a/internal/runbits/runtime/rationalize.go b/internal/runbits/runtime/rationalize.go index 60426f9850..3feac4ac8d 100644 --- a/internal/runbits/runtime/rationalize.go +++ b/internal/runbits/runtime/rationalize.go @@ -122,8 +122,6 @@ func RationalizeSolveError(proj *project.Project, auth *auth.Auth, rerr *error) ) // We communicate buildplanner errors verbatim as the intend is that these are curated by the buildplanner - // TODO: Maybe we should just forward any error message here? - // This could simplify the graphQL error handling. case errors.As(*rerr, &buildPlannerErr): *rerr = errs.WrapUserFacing(*rerr, buildPlannerErr.LocaleError(), diff --git a/pkg/buildscript/buildscript.go b/pkg/buildscript/buildscript.go index 3298f5a290..fcaf7981c7 100644 --- a/pkg/buildscript/buildscript.go +++ b/pkg/buildscript/buildscript.go @@ -16,8 +16,6 @@ import ( // methods that are easy to understand and work with. type BuildScript struct { raw *rawBuildScript - - atTime *time.Time // set after initial read } func init() { @@ -44,11 +42,11 @@ func New() *BuildScript { } func (b *BuildScript) AtTime() *time.Time { - return b.atTime + return b.raw.AtTime } func (b *BuildScript) SetAtTime(t time.Time) { - b.atTime = &t + b.raw.AtTime = &t } func (b *BuildScript) Equals(other *BuildScript) (bool, error) { diff --git a/pkg/buildscript/marshal.go b/pkg/buildscript/marshal.go index 173d7b7c34..c647616f47 100644 --- a/pkg/buildscript/marshal.go +++ b/pkg/buildscript/marshal.go @@ -28,9 +28,9 @@ const ( func (b *BuildScript) Marshal() ([]byte, error) { buf := strings.Builder{} - if b.atTime != nil { + if b.raw.AtTime != nil { buf.WriteString(assignmentString( - &assignment{atTimeKey, newString(b.atTime.Format(strfmt.RFC3339Millis))})) + &assignment{atTimeKey, newString(b.raw.AtTime.Format(strfmt.RFC3339Millis))})) buf.WriteString("\n") } diff --git a/pkg/buildscript/marshal_buildexpression.go b/pkg/buildscript/marshal_buildexpression.go index 15a6f62ef7..cb8a152803 100644 --- a/pkg/buildscript/marshal_buildexpression.go +++ b/pkg/buildscript/marshal_buildexpression.go @@ -41,7 +41,7 @@ func (b *BuildScript) MarshalBuildExpression() ([]byte, error) { if err != nil { return nil, errs.Wrap(err, "Invalid timestamp: %s", strValue(value)) } - b.atTime = ptr.To(time.Time(atTime)) + b.raw.AtTime = ptr.To(time.Time(atTime)) continue // do not include this custom assignment in the let block case mainKey: key = inKey // rename diff --git a/pkg/buildscript/raw.go b/pkg/buildscript/raw.go index 51f6ede04f..8a2dbc3149 100644 --- a/pkg/buildscript/raw.go +++ b/pkg/buildscript/raw.go @@ -3,6 +3,7 @@ package buildscript import ( "strconv" "strings" + "time" "github.com/ActiveState/cli/internal/rtutils/ptr" "github.com/brunoga/deep" @@ -11,6 +12,8 @@ import ( // Tagged fields will be filled in by Participle. type rawBuildScript struct { Assignments []*assignment `parser:"@@+"` + + AtTime *time.Time // set after initial read } // clone is meant to facilitate making modifications to functions at marshal time. The idea is that these modifications diff --git a/pkg/buildscript/unmarshal.go b/pkg/buildscript/unmarshal.go index 4ae616089f..ffbab27f9e 100644 --- a/pkg/buildscript/unmarshal.go +++ b/pkg/buildscript/unmarshal.go @@ -32,7 +32,6 @@ func Unmarshal(data []byte) (*BuildScript, error) { } // Extract 'at_time' value from the list of assignments, if it exists. - var atTime *time.Time for i, assignment := range raw.Assignments { key := assignment.Key value := assignment.Value @@ -43,11 +42,11 @@ func Unmarshal(data []byte) (*BuildScript, error) { if value.Str == nil { break } - parsedAtTime, err := strfmt.ParseDateTime(strValue(value)) + atTime, err := strfmt.ParseDateTime(strValue(value)) if err != nil { return nil, errs.Wrap(err, "Invalid timestamp: %s", strValue(value)) } - atTime = ptr.To(time.Time(parsedAtTime)) + raw.AtTime = ptr.To(time.Time(atTime)) break } @@ -61,5 +60,5 @@ func Unmarshal(data []byte) (*BuildScript, error) { seen[assignment.Key] = true } - return &BuildScript{raw, atTime}, nil + return &BuildScript{raw}, nil } diff --git a/pkg/buildscript/unmarshal_buildexpression.go b/pkg/buildscript/unmarshal_buildexpression.go index f4639aac5f..77af730583 100644 --- a/pkg/buildscript/unmarshal_buildexpression.go +++ b/pkg/buildscript/unmarshal_buildexpression.go @@ -5,7 +5,9 @@ import ( "sort" "strconv" "strings" + "time" + "github.com/go-openapi/strfmt" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -68,8 +70,13 @@ func (b *BuildScript) UnmarshalBuildExpression(data []byte) error { // Extract the 'at_time' from the solve node, if it exists, and change its value to be a // reference to "$at_time", which is how we want to show it in AScript format. if atTimeNode, err := b.getSolveAtTimeValue(); err == nil && atTimeNode.Str != nil && !strings.HasPrefix(strValue(atTimeNode), `$`) { + atTime, err := strfmt.ParseDateTime(strValue(atTimeNode)) + if err != nil { + return errs.Wrap(err, "Invalid timestamp: %s", strValue(atTimeNode)) + } atTimeNode.Str = nil atTimeNode.Ident = ptr.To("at_time") + b.raw.AtTime = ptr.To(time.Time(atTime)) } else if err != nil { return errs.Wrap(err, "Could not get at_time node") } diff --git a/pkg/platform/model/buildplanner/buildscript.go b/pkg/platform/model/buildplanner/buildscript.go index 52f038ee5f..28bd980ffc 100644 --- a/pkg/platform/model/buildplanner/buildscript.go +++ b/pkg/platform/model/buildplanner/buildscript.go @@ -52,10 +52,10 @@ func (b *BuildPlanner) GetBuildScript(commitID string) (*buildscript.BuildScript } script := buildscript.New() + script.SetAtTime(time.Time(resp.Commit.AtTime)) if err := script.UnmarshalBuildExpression(resp.Commit.Expression); err != nil { return nil, errs.Wrap(err, "failed to parse build expression") } - script.SetAtTime(time.Time(resp.Commit.AtTime)) return script, nil }