Skip to content

fix(git): preserve repository errors for orchestrator retries - #781

Merged
behinddwalls merged 2 commits into
uber:mainfrom
Tmwakalasya:tuntu/fix-git-repo-errors
Oct 6, 2026
Merged

behinddwalls merged 2 commits into
uber:mainfrom
Tmwakalasya:tuntu/fix-git-repo-errors

Conversation

@Tmwakalasya

Copy link
Copy Markdown
Contributor

Why?

The Git change provider calls Repo.FetchTarget during request validation. The repository execution helper formats Git failures as plain strings, discarding the operation, process error, and cancellation identity. A temporary DNS failure therefore cannot reach the existing Git retry policy, and a cancelled command no longer matches context.Canceled. The orchestrator also omits the Git classifier from its primary pipeline.

What?

  • Use the existing gitexec.CommandFailure helper at the repository's raw-command and configuration-write boundaries, preserving error chains through contextual wrapping.
  • Register the existing Git classifier in the orchestrator. Recognized transient fetch failures become retryable dependency errors; authentication and unknown Git failures remain permanent. Shutdown cancellation remains retryable without dependency attribution.
  • Add regression coverage through real failed Git commands, pre-cancelled contexts, and Repo.FetchTarget with a local fake executable feeding the production classifier list. No remote Git service is contacted by these tests.

The existing classifier policy and public function signatures are unchanged. Configuration-write errors continue to add only the configuration key to their context, without including its value.

Test Plan

The new regression tests failed before the production changes and passed afterward.

  • Passed: go test -mod=readonly for platform/git/repo, platform/git/exec, platform/errs/..., the Git change provider, orchestrator server, validate controller, consumer, and pipeline packages.
  • The repository, Git change-provider, and orchestrator server suites also passed through ./tool/bazel run @rules_go//go -- test -mod=readonly, using the pinned Go 1.25.0 toolchain.
  • Passed: go vet -mod=readonly ./platform/git/repo ./service/submitqueue/orchestrator/server.
  • Passed: make fmt lint, make check-tidy check-gazelle, and git diff --check.
  • Git-dependent Go suites used SUBMITQUEUE_TEST_GIT=/usr/bin/git and ran outside the macOS sandbox, whose warnings otherwise contaminate existing fixtures' combined output.
  • The affected Bazel test targets were attempted, but zlib compilation rejected an absolute include for Xcode's SDKSettings.json; zero tests ran through those targets. Full-repository and Docker integration suites were not run.

Issue

Found while tracing Git change-provider failures; no existing issue linked.

@behinddwalls
behinddwalls enabled auto-merge October 6, 2026 17:47
@behinddwalls
behinddwalls added this pull request to the merge queue Oct 6, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 6, 2026
@Tmwakalasya

Tmwakalasya commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

The merge-queue run failed fetching the gRPC dependency from proxy.golang.org due to an HTTP/2 stream error. Queue integration tests never started. All PR checks passed; could you please re-enqueue it? @behinddwalls

@behinddwalls
behinddwalls added this pull request to the merge queue Oct 6, 2026
Merged via the queue into uber:main with commit 8ddc4c4 Oct 6, 2026
16 checks passed
@Tmwakalasya

Copy link
Copy Markdown
Contributor Author

Thank you @behinddwalls

This branch was previously deployed

1 inactive deployment
stack-rebase — 26c16434 Deployed Oct 6, 2026 by behinddwalls via Rebase Stack #565
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants