Make BazelBuildSystem error message complete on opaque exits#158
Merged
Conversation
CI run 25243519727 surfaced a snapshotBazel failure where bazel exited 7 with both stderr and stdout empty. The pre-existing error formatter in `runBazel` — `output.stderr.isEmpty ? output.stdout : output.stderr` — discarded one stream by design and yielded an empty diagnostic when both were empty: the test failure message was just "Project build failed (exit code 7):" with nothing after the colon, leaving no way to determine whether the failure was a corrupt bazel binary, a concurrent workspace lock, an analysis failure with output redirected, or something else. The rerun of the same SHA passed, so the failure was transient — but silent-flake conditions in Bazel are common enough (server crashes, remote cache evictions mid-fetch, partial bazelisk downloads, file descriptor exhaustion) that we will encounter this again. When we do, we want the test failure to point at the offending command rather than just at the absence of one. The new diagnostic carries: - the bazel command and arguments that failed - the working directory (projectRoot) - the captured stderr (or `(empty)` if truly silent) - the captured stdout (or `(empty)` if truly silent) No behavior change on the happy path; no public-API change to `BuildSystemError.buildFailed`. Verified locally: `swift test --filter SnapshotCommandTests/snapshotBazel` passes in 4.8s.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CI run 25243519727 surfaced a
snapshotBazelfailure where bazel exited 7 with both stderr and stdout empty. The pre-existing error formatter inrunBazel:discarded one stream by design and yielded an empty diagnostic when both were empty. The test failure looked like:
— with nothing after the colon, leaving no way to tell whether the failure was a corrupt bazel binary, a concurrent workspace lock, an analysis failure with output redirected, or something else.
The rerun of the same SHA passed, so this particular instance was transient. But silent-flake conditions in Bazel (server crashes, remote-cache evictions mid-fetch, partial bazelisk downloads, fd exhaustion) are common enough that we'll see it again. When we do, the test failure should point at the offending command, not at the absence of one.
Change
The thrown
BuildSystemError.buildFailednow carries:projectRoot)(empty)if truly silent)(empty)if truly silent)No behavior change on the happy path. No public-API change to
BuildSystemError.buildFailed— the diagnostic is composed into the existingstderr:field.Why not also try to fix the underlying flake?
Without an actual reproduction we'd be speculating (version mismatch? concurrent jobs? remote cache state?). The diagnostic improvement is the load-bearing fix: the next occurrence will be self-describing instead of opaque, and we can address the actual root cause then. Adding speculative defensive layers (retry-on-fail, isolated
--output_user_root) without evidence risks masking legitimate build failures.Test plan
swift buildcleanswift test --filter SnapshotCommandTests/snapshotBazelpasses (4.8s) — happy path unaffected