Skip to content

fix(buildkite): handle null build metadata without panicking - #834

Open
Tmwakalasya wants to merge 5 commits into
uber:mainfrom
Tmwakalasya:tuntu/fix-buildkite-null-metadata
Open

Tmwakalasya wants to merge 5 commits into
uber:mainfrom
Tmwakalasya:tuntu/fix-buildkite-null-metadata

Conversation

@Tmwakalasya

Copy link
Copy Markdown
Contributor

Why?

A Buildkite response containing JSON null in the metadata environment variable makes DecodeMetadataEnv return a nil map without an error. Both the SubmitQueue and Stovepipe runners then panic when Status adds the build URL to that map.

This is a defensive response-handling fix: the normal Trigger path omits absent metadata rather than generating null.

What?

Normalize a successfully decoded nil map to an empty, writable map in the shared decoder. Preserve the existing decode-error behavior and document the successful-return contract.

Expand the empty-metadata test to cover absent metadata, an empty string, an empty object, JSON null, and null surrounded by whitespace. Each successful result must support adding metadata. Existing populated-map and malformed-JSON coverage remains in place.

Test Plan

  • The new null cases fail against the original decoder and pass with the fix.
  • Passed all three affected Bazel targets: ./tool/bazel test //platform/buildkite:go_default_test //submitqueue/extension/buildrunner/buildkite:go_default_test //stovepipe/extension/buildrunner/buildkite:go_default_test --test_output=errors.
  • Passed go vet -mod=readonly ./platform/buildkite ./submitqueue/extension/buildrunner/buildkite ./stovepipe/extension/buildrunner/buildkite.
  • Passed make fmt lint check-tidy check-gazelle and git diff --cached --check.
  • Reran the original local SubmitQueue Status diagnostic using an in-memory HTTP transport: empty-object, malformed-JSON, and null metadata all return the expected status and build URL without a panic. This diagnostic is separate from the PR's regression test.
  • Full-repository and integration suites were not run; verification focused on the shared decoder and both Buildkite adapters.

Issue

No linked issue. Found while tracing Buildkite metadata decoding and status polling.

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.

2 participants