Skip to content

fix: tolerate stderr-polluted moduleGraphJson in ModuleGraphParser#336

Open
rdark wants to merge 1 commit intoTinder:masterfrom
rdark:fix/stderr-pollution-tolerant-parse
Open

fix: tolerate stderr-polluted moduleGraphJson in ModuleGraphParser#336
rdark wants to merge 1 commit intoTinder:masterfrom
rdark:fix/stderr-pollution-tolerant-parse

Conversation

@rdark
Copy link
Copy Markdown

@rdark rdark commented Apr 27, 2026

Partial fix for #335

bazel-diff 17.0.1..18.0.5 configured BazelModService.getModuleGraphJson() with stdout=CAPTURE, stderr=CAPTURE. In Process.kt, captureAll triggers ProcessBuilder.redirectErrorStream(true), physically merging stderr into stdout. The captured moduleGraphJson therefore contained bazel's INFO lines ("INFO: Invocation ID: ...", "Loading: 0 packages loaded", etc.) prepended to the actual JSON output.

PR #330 (shipped in v18.1.0) correctly switched stderr to SILENT but broke format compatibility: any CI pipeline re-using a base graph from 17.0.1..18.0.5 and a head graph from 18.1.0+ hits parseModuleGraph's try/catch on the polluted side, gets emptyMap(), and cascades into findChangedModules reporting every head-side module as "added". The downstream queryTargetsDependingOnModules then spawns thousands of bazel query rdeps(...) subprocesses.

Make parseModuleGraph tolerant: attempt the whole-string parse first, and on failure retry from the first '{' to end-of-string. On second failure fall through to the existing emptyMap() behaviour. Clean input continues to parse in one attempt with no extra allocation.

Tests:

  • ModuleGraphParserTest: positive case for stderr-prefixed input.
  • StderrPollutionRegressionTest: end-to-end regression guard covering parse, findChangedModules, and the CalculateImpactedTargetsInteractor dispatch path. Confirms the cross-version comparison now short-circuits to computeSimpleImpactedTargets instead of the rdeps fan-out.

bazel-diff 17.0.1..18.0.5 configured BazelModService.getModuleGraphJson()
with stdout=CAPTURE, stderr=CAPTURE. In Process.kt, captureAll triggers
ProcessBuilder.redirectErrorStream(true), physically merging stderr into
stdout. The captured moduleGraphJson therefore contained bazel's INFO
lines ("INFO: Invocation ID: ...", "Loading: 0 packages loaded", etc.)
prepended to the actual JSON output.

PR Tinder#330 (shipped in v18.1.0) correctly switched stderr to SILENT but
broke format compatibility: any CI pipeline re-using a base graph from
17.0.1..18.0.5 and a head graph from 18.1.0+ hits parseModuleGraph's
try/catch on the polluted side, gets emptyMap(), and cascades into
findChangedModules reporting every head-side module as "added". The
downstream queryTargetsDependingOnModules then spawns thousands of
bazel query rdeps(...) subprocesses.

Make parseModuleGraph tolerant: attempt the whole-string parse first,
and on failure retry from the first '{' to end-of-string. On second
failure fall through to the existing emptyMap() behaviour. Clean input
continues to parse in one attempt with no extra allocation.

Tests:
- ModuleGraphParserTest: positive case for stderr-prefixed input.
- StderrPollutionRegressionTest: end-to-end regression guard covering
  parse, findChangedModules, and the CalculateImpactedTargetsInteractor
  dispatch path. Confirms the cross-version comparison now short-circuits
  to computeSimpleImpactedTargets instead of the rdeps fan-out.
@rdark rdark force-pushed the fix/stderr-pollution-tolerant-parse branch from 65f988e to a3cb64f Compare April 27, 2026 23:33
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