vbhanuchander-lang opened a new pull request, #9026:
URL: https://github.com/apache/devlake/pull/9026
### Summary
A Jira search 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 — currently fails the whole `collectIssues` subtask. On
a large Jira instance that throws away an otherwise complete sync because of a
single bad page, typically during deep pagination with changelog expansion.
This adds `JIRA_SKIP_UNPARSEABLE_ISSUES`:
- **Default `false`** — unchanged behaviour: the error surfaces and the
subtask fails.
- **`true`** — the offending page is logged with its URL and body size, then
skipped, and collection continues.
The flag is read through the same `taskCtx.GetConfigReader().GetBool(...)`
pattern that `JIRA_JQL_AUTO_FULL_REFRESH` already uses in this plugin, and is
logged once at the start of collection when enabled.
Only parse failures on otherwise-successful responses are affected. Non-2xx
statuses never reach the response parser and keep their existing retry
behaviour, as the issue requested.
The V2 (`api/2/search`) and V3 (`api/3/search/jql`) response parsers were
byte-identical, so rather than duplicating the change they now share one
`parseIssuesResponse` helper — a net reduction in duplicated code. A skipped
page returns an empty non-nil slice, so the collector records "this page had no
rows" instead of treating the result as absent.
### Does this close any open issues?
Closes #8949
### Screenshots
N/A — behavioural change with no UI surface.
### Other Information
**Tests.** Nine cases added to `issue_collector_test.go`, following the
existing table-driven style: valid pages under both flag states, the
HTML-error-page and truncated-body variants under both flag states, a valid
page with no `issues` key, and the empty-vs-nil distinction on the skip path.
Plus a small `responseUrl` test for nil-safety.
One wrinkle worth flagging: `unithelper.DummyLogger()` stubs `Warn` with two
arguments, but the real signature is `Warn(err, format, a ...interface{})`,
which mockery records as three. The test adds the variadic stub locally rather
than changing the shared helper or making the production call less idiomatic —
variadic `logger.Warn` is used widely across the plugins.
**Verification.** `go test ./plugins/jira/...` passes (`plugins/jira/tasks`
ok), `gofmt` clean, `go vet` clean for the changed files. Across the repo's
unit gate, the only failures I see locally are the `gitextractor` packages
failing to *build* because `pkg-config`/libgit2 isn't installed on my machine —
unrelated to this change. `e2e` is excluded from the unit gate and needs
`E2E_DB_URL`.
**Documentation.** I have not added docs here, matching the existing
precedent: `JIRA_JQL_AUTO_FULL_REFRESH` and the other plugin-level flags are
not documented in this repo, and `env.example` carries only global settings.
Happy to send a companion PR to the website repo if that is the preferred home
for it.
**Scope question for maintainers.** I have implemented this in the Jira
plugin as the issue asks, but the same "2xx with an unparseable body" failure
can affect any collector. If you would rather see it handled generically in the
shared collector helper, I am glad to rework it.
**Coordination.** @Murad-Suleymanov opened the issue and mentioned they were
willing to submit a PR. I commented on the issue before opening this and will
happily close it if they already have work in progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
