Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

gitserver: Fix some cases of RevNotFound in CommitLog#63063

Merged
eseliger merged 1 commit into
mainfrom
es/06-04-gitserverfixsomecasesofrevnotfoundincommitlog
Jun 4, 2024
Merged

gitserver: Fix some cases of RevNotFound in CommitLog#63063
eseliger merged 1 commit into
mainfrom
es/06-04-gitserverfixsomecasesofrevnotfoundincommitlog

Conversation

@eseliger

@eseliger eseliger commented Jun 4, 2024

Copy link
Copy Markdown
Member

There are so many different error messages this can return .. we seem to have missed a few that we need to map to RevisionNotFound, found in dotcom gitserver logs.

Test plan:

Added tests for the cases.

@cla-bot cla-bot Bot added the cla-signed label Jun 4, 2024
@github-actions github-actions Bot added team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all labels Jun 4, 2024

eseliger commented Jun 4, 2024

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @eseliger and the rest of your teammates on Graphite Graphite

@eseliger eseliger marked this pull request as ready for review June 4, 2024 06:54
@eseliger eseliger requested a review from a team June 4, 2024 06:54

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this would be easier to read if this was all extracted to a for loop

Suggested change
if bytes.Contains(e.Stderr, []byte("not a tree object")) ||
var errMessages = []string{
"not a tree object",
"fatal: bad object",
"fatal: ambiguous argument",
"unknown revision or path not in the working tree.",
"fatal: Invalid revision range",
"fatal: bad revision",
}
for _, message := range errMessages {
if bytes.Contains(e.Stderr, []byte(message)) {
return &gitdomain.RevisionNotFoundError{Repo: it.repoName, Spec: it.spec}
}
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

nice, thanks!

There are so many different error messages this can return .. we seem to have missed a few that we need to map to RevisionNotFound, found in dotcom gitserver logs.

Test plan:

Added tests for the cases.
@eseliger eseliger force-pushed the es/06-04-gitserverfixsomecasesofrevnotfoundincommitlog branch from 76639b4 to 10b3af2 Compare June 4, 2024 20:52
@eseliger eseliger merged commit 7d35662 into main Jun 4, 2024
@eseliger eseliger deleted the es/06-04-gitserverfixsomecasesofrevnotfoundincommitlog branch June 4, 2024 21:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants