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

Support user-facing BuildPlanner errors #2719

Merged
merged 10 commits into from
Aug 23, 2023
Merged

Support user-facing BuildPlanner errors #2719

merged 10 commits into from
Aug 23, 2023

Conversation

MDrakos
Copy link
Member

@MDrakos MDrakos commented Aug 21, 2023

StoryDX-2072 Support build planer user-facing errors

While working on this PR, a better way to handle the various error types coming from the BuildPlanner surfaced. This PR updates our error handling in this regard.

@github-actions github-actions bot changed the base branch from master to version/0-41-0-RC1 August 21, 2023 22:25
@MDrakos MDrakos marked this pull request as ready for review August 22, 2023 16:44
@MDrakos MDrakos requested a review from Naatan August 22, 2023 16:44

func ProcessCommitError(commit *Commit, fallbackMessage string) error {
if commit.Error == nil {
return errs.New(fallbackMessage)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a scenario that we expect to happen? We might want to present something localized if that's the case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, if this happens we've encountered an error type but the message wasn't set which would be an invalid according to the build planner's schema.

pkg/platform/api/buildplanner/model/buildplan.go Outdated Show resolved Hide resolved
pkg/platform/api/buildplanner/model/buildplan.go Outdated Show resolved Hide resolved
pkg/platform/api/buildplanner/model/buildplan.go Outdated Show resolved Hide resolved
pkg/platform/api/buildplanner/model/buildplan.go Outdated Show resolved Hide resolved
pkg/platform/api/buildplanner/model/buildplan.go Outdated Show resolved Hide resolved
}

return b.Commit.CommitID, nil
return errs.New(fallbackMessage)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to silence this one (input error)?

Copy link
Member Author

@MDrakos MDrakos Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, if we have to use the fallback message we've encountered an error type that we aren't handling so we should update these functions.

@MDrakos MDrakos requested a review from Naatan August 22, 2023 20:33
@Naatan Naatan merged commit b2a3bc6 into version/0-41-0-RC1 Aug 23, 2023
5 of 6 checks passed
@Naatan Naatan deleted the DX-2072 branch August 23, 2023 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants