This is an automated email from the ASF dual-hosted git repository.
klesh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/devlake.git
The following commit(s) were added to refs/heads/main by this push:
new e3a42717b feat(jira): add JIRA_SKIP_UNPARSEABLE_ISSUES to survive
unparseable issue pages (#9026)
e3a42717b is described below
commit e3a42717bdba6761bd43c38fadbf6938a0fcd7e7
Author: vbhanuchander-lang <[email protected]>
AuthorDate: Mon Aug 3 02:50:16 2026 -0400
feat(jira): add JIRA_SKIP_UNPARSEABLE_ISSUES to survive unparseable issue
pages (#9026)
* feat(jira): add JIRA_SKIP_UNPARSEABLE_ISSUES to survive unparseable issue
pages
A page that comes back with a successful status but a body that is not the
expected JSON - a truncated payload, or an HTML error page substituted by a
proxy - fails the whole collectIssues subtask. On a large Jira instance that
discards an otherwise complete sync because of a single bad page, typically
during deep pagination with changelog expansion.
Add JIRA_SKIP_UNPARSEABLE_ISSUES, read through the same
taskCtx.GetConfigReader().GetBool() pattern that JIRA_JQL_AUTO_FULL_REFRESH
already uses in this plugin. It defaults to false, preserving the current
behaviour of surfacing the error and failing the subtask. When enabled, the
offending page is logged with its URL and body size and skipped, and
collection
continues.
Only parse failures on otherwise-successful responses are affected. Non-2xx
statuses never reach the response parser and keep their existing retry
behaviour.
The V2 (api/2/search) and V3 (api/3/search/jql) response parsers were byte
identical, so both now share a single parseIssuesResponse helper rather than
carrying two copies of the change. A skipped page yields an empty non-nil
slice, so the collector records a page with no rows instead of treating the
result as absent.
Signed-off-by: Bhanu Chander Vallabaneni <[email protected]>
* docs(jira): document JIRA_SKIP_UNPARSEABLE_ISSUES in env.example
Requested in review on #9026.
Signed-off-by: Bhanu Chander Vallabaneni <[email protected]>
---------
Signed-off-by: Bhanu Chander Vallabaneni <[email protected]>
---
backend/plugins/jira/tasks/issue_collector.go | 78 +++++++++-----
backend/plugins/jira/tasks/issue_collector_test.go | 118 +++++++++++++++++++++
env.example | 10 ++
3 files changed, 178 insertions(+), 28 deletions(-)
diff --git a/backend/plugins/jira/tasks/issue_collector.go
b/backend/plugins/jira/tasks/issue_collector.go
index fe1939b72..997a73685 100644
--- a/backend/plugins/jira/tasks/issue_collector.go
+++ b/backend/plugins/jira/tasks/issue_collector.go
@@ -30,6 +30,7 @@ import (
"github.com/apache/incubator-devlake/core/dal"
"github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
"github.com/apache/incubator-devlake/plugins/jira/models"
@@ -95,12 +96,17 @@ func CollectIssues(taskCtx plugin.SubTaskContext)
errors.Error {
pageSize = 100
}
+ skipUnparseable :=
taskCtx.GetConfigReader().GetBool("JIRA_SKIP_UNPARSEABLE_ISSUES")
+ if skipUnparseable {
+ logger.Info("JIRA_SKIP_UNPARSEABLE_ISSUES is enabled,
unparseable issue pages will be skipped")
+ }
+
if strings.EqualFold(string(data.JiraServerInfo.DeploymentType),
string(models.DeploymentServer)) {
logger.Info("Using api/2/search for JIRA Server issue
collection")
- err = setupIssueV2Collector(apiCollector, data, filterJql,
pageSize)
+ err = setupIssueV2Collector(apiCollector, data, filterJql,
pageSize, skipUnparseable, logger)
} else {
logger.Info("Using api/3/search/jql for JIRA Cloud issue
collection")
- err = setupIssueV3Collector(apiCollector, data, filterJql,
pageSize)
+ err = setupIssueV3Collector(apiCollector, data, filterJql,
pageSize, skipUnparseable, logger)
}
if err != nil {
return err
@@ -176,7 +182,45 @@ func buildFilterJQL(filterId string, extraJql string,
incrementalJql string) str
return strings.Join(conditions, " AND ") + " " + orderBy
}
-func setupIssueV2Collector(apiCollector *api.StatefulApiCollector, data
*JiraTaskData, filterJql string, pageSize int) errors.Error {
+// parseIssuesResponse extracts the `issues` array from a Jira search response.
+//
+// A response that arrived with a successful status but carries a body which
is not the
+// expected JSON - a truncated payload, or an HTML error page substituted by a
proxy -
+// normally fails the whole collectIssues subtask, discarding an otherwise
complete sync
+// because of a single page. When skipUnparseable is set the page is logged
and skipped
+// instead. Non-2xx responses never reach this point; they are handled by the
retry logic
+// in the API client.
+func parseIssuesResponse(res *http.Response, skipUnparseable bool, logger
log.Logger) ([]json.RawMessage, errors.Error) {
+ blob, err := io.ReadAll(res.Body)
+ if err != nil {
+ return nil, errors.Convert(err)
+ }
+ var body struct {
+ Issues []json.RawMessage `json:"issues"`
+ }
+ if err := json.Unmarshal(blob, &body); err != nil {
+ if !skipUnparseable {
+ return nil, errors.Convert(err)
+ }
+ if logger != nil {
+ logger.Warn(err, "skipping unparseable issue page from
%s (%d bytes)", responseUrl(res), len(blob))
+ }
+ return []json.RawMessage{}, nil
+ }
+ return body.Issues, nil
+}
+
+// responseUrl reports the request URL behind a response, for log messages.
The request is
+// always populated on responses returned by the API client, but a hand-built
response in a
+// test may not carry one.
+func responseUrl(res *http.Response) string {
+ if res == nil || res.Request == nil || res.Request.URL == nil {
+ return "unknown url"
+ }
+ return res.Request.URL.String()
+}
+
+func setupIssueV2Collector(apiCollector *api.StatefulApiCollector, data
*JiraTaskData, filterJql string, pageSize int, skipUnparseable bool, logger
log.Logger) errors.Error {
return apiCollector.InitCollector(api.ApiCollectorArgs{
ApiClient: data.ApiClient,
PageSize: pageSize,
@@ -192,23 +236,12 @@ func setupIssueV2Collector(apiCollector
*api.StatefulApiCollector, data *JiraTas
GetTotalPages: GetTotalPagesFromResponse,
Concurrency: 10,
ResponseParser: func(res *http.Response) ([]json.RawMessage,
errors.Error) {
- var data struct {
- Issues []json.RawMessage `json:"issues"`
- }
- blob, err := io.ReadAll(res.Body)
- if err != nil {
- return nil, errors.Convert(err)
- }
- err = json.Unmarshal(blob, &data)
- if err != nil {
- return nil, errors.Convert(err)
- }
- return data.Issues, nil
+ return parseIssuesResponse(res, skipUnparseable, logger)
},
})
}
-func setupIssueV3Collector(apiCollector *api.StatefulApiCollector, data
*JiraTaskData, filterJql string, pageSize int) errors.Error {
+func setupIssueV3Collector(apiCollector *api.StatefulApiCollector, data
*JiraTaskData, filterJql string, pageSize int, skipUnparseable bool, logger
log.Logger) errors.Error {
return apiCollector.InitCollector(api.ApiCollectorArgs{
ApiClient: data.ApiClient,
PageSize: pageSize,
@@ -226,18 +259,7 @@ func setupIssueV3Collector(apiCollector
*api.StatefulApiCollector, data *JiraTas
return query, nil
},
ResponseParser: func(res *http.Response) ([]json.RawMessage,
errors.Error) {
- var data struct {
- Issues []json.RawMessage `json:"issues"`
- }
- blob, err := io.ReadAll(res.Body)
- if err != nil {
- return nil, errors.Convert(err)
- }
- err = json.Unmarshal(blob, &data)
- if err != nil {
- return nil, errors.Convert(err)
- }
- return data.Issues, nil
+ return parseIssuesResponse(res, skipUnparseable, logger)
},
})
}
diff --git a/backend/plugins/jira/tasks/issue_collector_test.go
b/backend/plugins/jira/tasks/issue_collector_test.go
index 50c66947c..098196f93 100644
--- a/backend/plugins/jira/tasks/issue_collector_test.go
+++ b/backend/plugins/jira/tasks/issue_collector_test.go
@@ -18,10 +18,17 @@ limitations under the License.
package tasks
import (
+ "io"
+ "net/http"
+ "net/url"
+ "strings"
"testing"
"time"
+ "github.com/apache/incubator-devlake/helpers/unithelper"
+ mocklog "github.com/apache/incubator-devlake/mocks/core/log"
"github.com/apache/incubator-devlake/plugins/jira/models"
+ "github.com/stretchr/testify/mock"
)
func Test_buildJQL(t *testing.T) {
@@ -142,6 +149,117 @@ func Test_buildFilterJQL(t *testing.T) {
}
}
+func Test_parseIssuesResponse(t *testing.T) {
+ const twoIssues = `{"issues":[{"id":"1"},{"id":"2"}]}`
+ // A proxy returning an HTML error page under a 200, the case reported
in #8949.
+ const htmlPage = `<html><head><title>502 Bad
Gateway</title></head></html>`
+ // A response truncated mid-flight, so the JSON never terminates.
+ const truncated = `{"issues":[{"id":"1"},{"id":`
+
+ makeResponse := func(body string) *http.Response {
+ return &http.Response{
+ Body: io.NopCloser(strings.NewReader(body)),
+ Request: &http.Request{URL: &url.URL{Scheme: "https",
Host: "jira.example.com", Path: "/rest/api/2/search"}},
+ }
+ }
+
+ tests := []struct {
+ name string
+ body string
+ skipUnparseable bool
+ wantCount int
+ wantErr bool
+ // wantSkipped marks the cases that took the skip path, which
must yield an
+ // empty non-nil slice so the collector records "this page had
no rows" rather
+ // than treating the result as absent.
+ wantSkipped bool
+ }{
+ {
+ name: "valid page is parsed",
+ body: twoIssues,
+ wantCount: 2,
+ },
+ {
+ name: "valid page is parsed when skipping is
enabled",
+ body: twoIssues,
+ skipUnparseable: true,
+ wantCount: 2,
+ },
+ {
+ name: "html body fails by default",
+ body: htmlPage,
+ wantErr: true,
+ },
+ {
+ name: "html body is skipped when enabled",
+ body: htmlPage,
+ skipUnparseable: true,
+ wantCount: 0,
+ wantSkipped: true,
+ },
+ {
+ name: "truncated body fails by default",
+ body: truncated,
+ wantErr: true,
+ },
+ {
+ name: "truncated body is skipped when
enabled",
+ body: truncated,
+ skipUnparseable: true,
+ wantCount: 0,
+ wantSkipped: true,
+ },
+ {
+ name: "valid page without an issues key yields no
issues",
+ body: `{"total":0}`,
+ wantCount: 0,
+ },
+ }
+
+ // unithelper.DummyLogger stubs Warn with two arguments, but the real
signature is
+ // Warn(err, format, a ...interface{}), which mockery records as three.
Add the
+ // variadic form so the skip path can log.
+ newLogger := func() *mocklog.Logger {
+ logger := unithelper.DummyLogger()
+ logger.On("Warn", mock.Anything, mock.Anything,
mock.Anything).Maybe()
+ return logger
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ got, err := parseIssuesResponse(makeResponse(tt.body),
tt.skipUnparseable, newLogger())
+ if (err != nil) != tt.wantErr {
+ t.Errorf("parseIssuesResponse() error = %v,
wantErr %v", err, tt.wantErr)
+ return
+ }
+ if tt.wantErr {
+ return
+ }
+ if len(got) != tt.wantCount {
+ t.Errorf("parseIssuesResponse() returned %d
issues, want %d", len(got), tt.wantCount)
+ }
+ if tt.wantSkipped && got == nil {
+ t.Error("parseIssuesResponse() returned nil for
a skipped page, want an empty slice")
+ }
+ })
+ }
+}
+
+func Test_responseUrl(t *testing.T) {
+ withUrl := &http.Response{
+ Request: &http.Request{URL: &url.URL{Scheme: "https", Host:
"jira.example.com", Path: "/rest/api/2/search"}},
+ }
+ if got, want := responseUrl(withUrl),
"https://jira.example.com/rest/api/2/search";; got != want {
+ t.Errorf("responseUrl() = %v, want %v", got, want)
+ }
+ if got, want := responseUrl(&http.Response{}), "unknown url"; got !=
want {
+ t.Errorf("responseUrl() with no request = %v, want %v", got,
want)
+ }
+ if got, want := responseUrl(nil), "unknown url"; got != want {
+ t.Errorf("responseUrl(nil) = %v, want %v", got, want)
+ }
+}
+
func Test_renderExtraJQL(t *testing.T) {
makeData := func(boardId uint64, boardName string, _ string)
*JiraTaskData {
return &JiraTaskData{
diff --git a/env.example b/env.example
index 04e0ab0a5..b1de65982 100755
--- a/env.example
+++ b/env.example
@@ -87,6 +87,16 @@ USE_GO_GIT_IN_GIT_EXTRACTOR=false
SKIP_COMMIT_STAT=false
SKIP_COMMIT_FILES=true
+# In plugin jira, continue collecting when a page of issues cannot be parsed.
+# A page that returns a successful HTTP status but a body that is not the
+# expected JSON - a truncated payload, or an HTML error page substituted by a
+# proxy - normally fails the whole collectIssues subtask, discarding an
+# otherwise complete sync because of a single page. Set to true to log and skip
+# that page instead. Only parse failures on successful responses are affected;
+# HTTP error statuses keep their existing retry behaviour.
+##########################
+JIRA_SKIP_UNPARSEABLE_ISSUES=false
+
# Set if response error when requesting /connections/{connection_id}/test
should be wrapped or not
##########################
WRAP_RESPONSE_ERROR=
