Skip to content

Return empty feed instead of 204 from the DiscoJuice feeds endpoint - #1422

Merged
milanmajchrak merged 2 commits into
dtq-dev-9-basefrom
ufal/fix-discojuice-empty-feed-9-base
Sep 2, 2026
Merged

Return empty feed instead of 204 from the DiscoJuice feeds endpoint#1422
milanmajchrak merged 2 commits into
dtq-dev-9-basefrom
ufal/fix-discojuice-empty-feed-9-base

Conversation

@Kasinhou

@Kasinhou Kasinhou commented Sep 1, 2026

Copy link
Copy Markdown

Problem

The login and register pages throw an uncaught JS error on load, which fails the Playwright consoleErrors.spec.ts checks (2 pages × chromium/firefox/webkit = 6 failing tests):

TypeError: Cannot read properties of undefined (reading 'length')
    at n.merge (jquery-2.1.4.min.js)
    at Object.success (aai/discojuice/discojuice.js:62)

Root cause

The DiscoJuice IdP-discovery widget on those pages loads /server/api/discojuice/feeds?callback=dj_md_1 via JSONP and passes the response body straight to jQuery.merge().

ClarinDiscoJuiceFeedsController returned 204 No Content whenever the feed cache was blank:

if (StringUtils.isBlank(feedsContent)) {
    response.sendError(HttpServletResponse.SC_NO_CONTENT);
    return null;
}

That cache is populated only by ClarinDiscoJuiceFeedsUpdateScheduler, which downloads feeds only when shibboleth.discofeed.allowed is true — and that property defaults to false. So on a default deployment the endpoint always returns 204, the JSONP script tag executes with no body, the dj_md_1(...) callback never receives its argument, and jQuery.merge(list, undefined) reads undefined.length and throws.

A 204 fundamentally breaks the JSONP contract: the response body must be executable JS that calls the callback, so an empty body can never be consumed safely.

Fix

When there is no feed content, return an empty but valid feed instead of a 204:

  • with a callback → dj_md_1([])
  • without a callback → []

The callback now always fires with a well-formed (possibly empty) array, so the widget degrades gracefully (no extra IdPs listed) instead of crashing. Deployments that have shibboleth.discofeed.allowed enabled and real feed content are unaffected — that path is unchanged.

The now-unused HttpServletResponse parameter and its imports were removed along with the sendError branch.

Test

Added getDiscoFeedsWithoutContentReturnsEmptyFeed to ClarinDiscoJuiceFeedsControllerIT, which forces the empty-cache state and asserts 200 with dj_md_1([]) (with callback) and [] (without), instead of a 204. The existing getDiscoFeeds test (populated feed) still covers the normal path.

Verification

Reproduced live on dev-6.pc:8603: the error fires on /repository/login and /repository/register (both mount the widget) and not on /repository/home (no widget). The failing network call is GET /server/api/discojuice/feeds?callback=dj_md_1 → 204.

Note: the widget also makes external calls to store.discojuice.org that fail with ERR_NAME_NOT_RESOLVED in the sandboxed test env. That is unrelated environment noise — the 204 is what triggers the console error the tests assert on.

🤖 Generated with Claude Code

Matus Kasak and others added 2 commits September 1, 2026 14:03
When no discofeed content is cached — the default, since
`shibboleth.discofeed.allowed` is off — `/api/discojuice/feeds`
responded with `204 No Content`. The DiscoJuice IdP-discovery widget on
the login and register pages loads this endpoint via JSONP and passes
the body straight to `jQuery.merge()`. An empty body leaves the callback
argument undefined, so the widget throws
`TypeError: Cannot read properties of undefined (reading 'length')`
on every default deployment.

Respond with an empty but valid feed instead: `callback([])` when a
callback is given, `[]` otherwise. The JSONP callback then always fires
with a well-formed array and the widget no longer crashes; deployments
with feeds enabled are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@milanmajchrak
milanmajchrak merged commit b13cdd7 into dtq-dev-9-base Sep 2, 2026
17 of 20 checks passed
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.

3 participants