fix(azuredevops_go): handle empty/invalid timeline responses for YAML…#8839
Open
techdecline wants to merge 2 commits intoapache:mainfrom
Open
fix(azuredevops_go): handle empty/invalid timeline responses for YAML…#8839techdecline wants to merge 2 commits intoapache:mainfrom
techdecline wants to merge 2 commits intoapache:mainfrom
Conversation
…-broken builds Builds that fail due to YAML syntax errors in pipeline definitions return an empty or non-JSON body from the Timeline API (200 OK with no usable content). Previously this caused ParseRawMessageFromRecords to propagate a hard error that aborted the entire collectApiTimelineRecords subtask. Two complementary fixes: 1. Add ignoreInvalidTimelineResponse AfterResponse handler that supersedes ignoreDeletedBuilds: it still skips 404s (deleted builds) and now also skips responses with empty or non-JSON bodies by returning api.ErrIgnoreAndContinue, leaving the rest of the pipeline intact. The body is peeked and then restored via io.NopCloser so the downstream ResponseParser can still read it when the body is valid JSON. 2. Extend the DB query in CollectRecords to exclude builds with result = 'none' in addition to result = 'failed'. Builds broken by YAML syntax errors are categorised as 'none' by Azure DevOps (already mapped to RESULT_FAILURE in cicdBuildResultRule), so fetching their timeline is both unnecessary and the primary source of the bad responses. Using NOT IN ? with a []string slice follows the same pattern used in blueprint_helper.go. Unit tests added for ignoreInvalidTimelineResponse covering: 404, empty body, non-JSON body, valid JSON (nil error + body restored), and valid empty JSON object. Fixes: apache#8838 Signed-off-by: Cornelius Schuchardt <cornelius.schuchardt@beck.de>
dosubot
Bot
added
size:M
3 tasks
Contributor
|
Hi, can you fix the golang lint so I can merge it? |
The ignoreDeletedBuilds function was replaced by ignoreInvalidTimelineResponse in ci_cd_timeline_records_collector.go but was left behind in shared.go. This caused the 'unused' linter to fail with exit code 2.
Author
|
Hi @klesh , I just pushed a fix for the linting error. |
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
2 participants
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.
…-broken builds
Builds that fail due to YAML syntax errors in pipeline definitions return an empty or non-JSON body from the Timeline API (200 OK with no usable content). Previously this caused ParseRawMessageFromRecords to propagate a hard error that aborted the entire collectApiTimelineRecords subtask.
Two complementary fixes:
Add ignoreInvalidTimelineResponse AfterResponse handler that supersedes ignoreDeletedBuilds: it still skips 404s (deleted builds) and now also skips responses with empty or non-JSON bodies by returning api.ErrIgnoreAndContinue, leaving the rest of the pipeline intact. The body is peeked and then restored via io.NopCloser so the downstream ResponseParser can still read it when the body is valid JSON.
Extend the DB query in CollectRecords to exclude builds with result = 'none' in addition to result = 'failed'. Builds broken by YAML syntax errors are categorised as 'none' by Azure DevOps (already mapped to RESULT_FAILURE in cicdBuildResultRule), so fetching their timeline is both unnecessary and the primary source of the bad responses. Using NOT IN ? with a []string slice follows the same pattern used in blueprint_helper.go.
Unit tests added for ignoreInvalidTimelineResponse covering: 404, empty body, non-JSON body, valid JSON (nil error + body restored), and valid empty JSON object.
Fixes: #8838
pr-type/bug-fix,pr-type/feature-development, etc.Summary
What does this PR do?
Does this close any open issues?
Closes xx
Screenshots
Include any relevant screenshots here.
Other Information
Any other information that is important to this PR.