FWIW, the underlying patchdemo failure is tracked at https://gitlab.wikimedia.org/repos/ci-tools/patchdemo/-/issues/607.

The other issue seems to be that this should only happen for MediaWiki changes but not all changes.

It should happen for changes to all repos Patch Demo supports. The list is quite long and may change any time, so I don’t think it can be pre-filtered meaningfully. If nothing is broken, repos not supported by Patch Demo simply don’t display anything, similarly to patches in supported repos that don’t have any patch demos.

Thanks for the clarification. As of today I can confirm it happened in repo operations/puppet but that is not on the list above.

Thank you for the task! The issue is that patch demo can fail and the fetched json would end up being invalid since it is an error message and additionally patch demo does not necessarily sends a non 200 http response code. We had the same issue previously at T363355.

My best bet is the Gerrit Javascript plugin should catch and ignore any errors. Something to be adjusted in the JavaScript `fetch() (T363355#9741501). The fetching code is currently:

async fetch(change) {
  const url = `https://patchdemo.wmflabs.org/api.php?action=findwikis&change=${change.changeNumber}`;
  return fetch(url, { cache: 'no-store' })
    .then(resp => {
      if (!resp.ok) {
        throw new Error('HTTP ' + resp.status);
      }
      return resp.text();
    })
    .then(resp => {
      return this.parse(resp, change);
    })
    .catch( fetchError => {
      return {
        responseCode: /** @type {ResponseCode} */ ('ERROR'),
        errorMessage: fetchError,
      };
    });
}

Where this.parse has a call to JSON.parse(resp) which throws an exception whenever patch demo is not happy.

In T367155#9882210, @Dzahn wrote:

Thanks for the clarification. As of today I can confirm it happened in repo operations/puppet but that is not on the list above.

As I wrote, pre-filtering cannot be done in any meaningful way but asking Patch Demo if it supports the repo. However, if Patch Demo is broken, the pre-filtering would be broken as well anyway, and if it’s not broken, the pre-filtering would just make the code slower, so I don’t think it would make sense – and it doesn’t happen either.

In T367155#9883217, @hashar wrote:

My best bet is the Gerrit Javascript plugin should catch and ignore any errors.

The problem is that this would also mean that patches that do have patch demos would also do as if no patch demos existed.

The issue is that patch demo can fail and the fetched json would end up being invalid since it is an error message and additionally patch demo does not necessarily sends a non 200 http response code. We had the same issue previously at T363355.

By the way, in this case, it wasn’t invalid JSON: due to missing CORS headers, fetch itself failed, the response text wasn’t available to the Gerrit plugin at all.

Change #1042153 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.9] wm-patch-demo: silently ignore errors

https://gerrit.wikimedia.org/r/1042153

hashar opened https://gitlab.wikimedia.org/repos/ci-tools/patchdemo/-/merge_requests/619

Normalize API errors

Change #1042153 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.9] wm-patch-demo: silently ignore errors

https://gerrit.wikimedia.org/r/1042153

Mentioned in SAL (#wikimedia-operations) [2024-06-12T19:49:10Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@e4c49f9]: wm-patch-demo: silently ignore errors - T367155

Mentioned in SAL (#wikimedia-operations) [2024-06-12T19:49:17Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@e4c49f9]: wm-patch-demo: silently ignore errors - T367155 (duration: 00m 07s)