Skip to content

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

Open
Tmwakalasya wants to merge 1 commit into
uber:mainfrom
Tmwakalasya:tuntu/fix-git-repo-errors
Open

Tmwakalasya wants to merge 1 commit 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.

This branch has not been deployed

No deployments
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.

1 participant