fix(azuredevops_go): handle empty/invalid timeline responses for YAML… (#8839)

* fix(azuredevops_go): handle empty/invalid timeline responses for YAML-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: #8838
Signed-off-by: Cornelius Schuchardt <cornelius.schuchardt@beck.de>

* fix(azuredevops_go): remove unused ignoreDeletedBuilds function

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.

* fix: resolve govet non-constant format string warnings

Use constant format verbs instead of dynamic strings passed directly
as format arguments to avoid govet printf-family warnings:

- core/migration/linter: fmt.Errorf with string concat replaced by %s verb
- impls/logruslog: Warn/Error pass pre-formatted message via "%s" arg
  instead of using it directly as the format string

---------

Signed-off-by: Cornelius Schuchardt <cornelius.schuchardt@beck.de>

Author: techdecline

backend/core/migration/linter/main.go

@@ -51,7 +51,7 @@ func firstLineFromFile(path string) string {
 	for scanner.Scan() {
 		return scanner.Text()
 	}
-	panic(fmt.Errorf("empty file: " + path))
+	panic(fmt.Errorf("empty file: %s", path))
 }
 
 const (

backend/impls/logruslog/logger.go

@@ -72,11 +72,11 @@ func (l *DefaultLogger) Info(format string, a ...interface{}) {
 }
 
 func (l *DefaultLogger) Warn(err error, format string, a ...interface{}) {
-	l.Log(log.LOG_WARN, formatMessage(err, format, a...))
+	l.Log(log.LOG_WARN, "%s", formatMessage(err, format, a...))
 }
 
 func (l *DefaultLogger) Error(err error, format string, a ...interface{}) {
-	l.Log(log.LOG_ERROR, formatMessage(err, format, a...))
+	l.Log(log.LOG_ERROR, "%s", formatMessage(err, format, a...))
 }
 
 func (l *DefaultLogger) SetStream(config *log.LoggerStreamConfig) {

backend/plugins/azuredevops_go/tasks/ci_cd_timeline_records_collector.go

@@ -49,7 +49,7 @@ func CollectRecords(taskCtx plugin.SubTaskContext) errors.Error {
 	cursor, err := db.Cursor(
 		dal.Select("azuredevops_id"),
 		dal.From(models.AzuredevopsBuild{}.TableName()),
-		dal.Where("repository_id = ? and connection_id=? and result != ?", data.Options.RepositoryId, data.Options.ConnectionId, "failed"),
+		dal.Where("repository_id = ? and connection_id=? and result NOT IN ?", data.Options.RepositoryId, data.Options.ConnectionId, []string{"failed", "none"}),
 	)
 	if err != nil {
 		return err
@@ -68,7 +68,7 @@ func CollectRecords(taskCtx plugin.SubTaskContext) errors.Error {
 		UrlTemplate:        "{{ .Params.OrganizationId }}/{{ .Params.ProjectId }}/_apis/build/builds/{{ .Input.AzuredevopsId }}/Timeline?api-version=7.1",
 		Query:              BuildPaginator(true),
 		ResponseParser:     ParseRawMessageFromRecords,
-		AfterResponse:      ignoreDeletedBuilds, // Ignore the 404 response if builds are deleted during the collection
+		AfterResponse:      ignoreInvalidTimelineResponse, // Skip builds with missing/malformed timelines (e.g. YAML syntax errors)
 	})
 	if err != nil {
 		return err

backend/plugins/azuredevops_go/tasks/shared.go

@@ -18,14 +18,17 @@ limitations under the License.
 package tasks
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
+	"net/http"
+	"net/url"
+
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/models/domainlayer/devops"
 	"github.com/apache/incubator-devlake/core/plugin"
 	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"net/http"
-	"net/url"
 )
 
 // Build and TimeLine Record State and Result types can be found here:
@@ -145,9 +148,29 @@ func change203To401(res *http.Response) errors.Error {
 	return nil
 }
 
-func ignoreDeletedBuilds(res *http.Response) errors.Error {
+// ignoreInvalidTimelineResponse is an AfterResponse handler for the Timeline API.
+// It skips builds whose timeline response is missing or unparseable (e.g. builds
+// that failed due to a YAML syntax error never produce a usable timeline), instead
+// of aborting the entire subtask.
+func ignoreInvalidTimelineResponse(res *http.Response) errors.Error {
+	// Keep existing behaviour: treat 404 as a graceful skip (build was deleted).
 	if res.StatusCode == http.StatusNotFound {
 		return api.ErrIgnoreAndContinue
 	}
+
+	// Read the body so we can inspect it, then restore it for the ResponseParser.
+	body, err := io.ReadAll(res.Body)
+	_ = res.Body.Close()
+	if err != nil {
+		return api.ErrIgnoreAndContinue
+	}
+	res.Body = io.NopCloser(bytes.NewReader(body))
+
+	// An empty body or non-JSON body means the timeline is not available.
+	// Return ErrIgnoreAndContinue so the build is skipped without failing the subtask.
+	if len(body) == 0 || !json.Valid(body) {
+		return api.ErrIgnoreAndContinue
+	}
+
 	return nil
 }

backend/plugins/azuredevops_go/tasks/shared_test.go

@@ -0,0 +1,72 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package tasks
+
+import (
+	"bytes"
+	"io"
+	"net/http"
+	"testing"
+
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/stretchr/testify/assert"
+)
+
+func makeResponse(statusCode int, body string) *http.Response {
+	return &http.Response{
+		StatusCode: statusCode,
+		Body:       io.NopCloser(bytes.NewBufferString(body)),
+		Request:    &http.Request{},
+	}
+}
+
+func TestIgnoreInvalidTimelineResponse_404(t *testing.T) {
+	res := makeResponse(http.StatusNotFound, "")
+	err := ignoreInvalidTimelineResponse(res)
+	assert.Equal(t, api.ErrIgnoreAndContinue, err, "404 should return ErrIgnoreAndContinue")
+}
+
+func TestIgnoreInvalidTimelineResponse_EmptyBody(t *testing.T) {
+	res := makeResponse(http.StatusOK, "")
+	err := ignoreInvalidTimelineResponse(res)
+	assert.Equal(t, api.ErrIgnoreAndContinue, err, "empty body should return ErrIgnoreAndContinue")
+}
+
+func TestIgnoreInvalidTimelineResponse_NonJSONBody(t *testing.T) {
+	res := makeResponse(http.StatusOK, "not json at all")
+	err := ignoreInvalidTimelineResponse(res)
+	assert.Equal(t, api.ErrIgnoreAndContinue, err, "non-JSON body should return ErrIgnoreAndContinue")
+}
+
+func TestIgnoreInvalidTimelineResponse_ValidJSON_ReturnsNil(t *testing.T) {
+	validJSON := `{"records":[{"id":"abc","type":"Job","name":"Build"}]}`
+	res := makeResponse(http.StatusOK, validJSON)
+	err := ignoreInvalidTimelineResponse(res)
+	assert.Nil(t, err, "valid JSON body should return nil")
+
+	// Verify body is still readable after the handler restored it.
+	remaining, readErr := io.ReadAll(res.Body)
+	assert.NoError(t, readErr)
+	assert.Equal(t, validJSON, string(remaining), "body should be restored for downstream parser")
+}
+
+func TestIgnoreInvalidTimelineResponse_ValidEmptyJSONObject(t *testing.T) {
+	res := makeResponse(http.StatusOK, "{}")
+	err := ignoreInvalidTimelineResponse(res)
+	assert.Nil(t, err, "valid empty JSON object should return nil")
+}