OAI-PMH error handling: meaningful errors in XML for verb processing

[pdurbin]

What this PR does / why we need it:

Currently, certain OAI requests, especially related to verbs or the absence of a verb, result in a 500 error and no meaningful information for the end user. Plus the poor sysadmin gets an ugly stacktrace in server.log.

As of this PR, we send XML responses with meaningful errors, like this:

/oai?foo=bar will show "No argument 'verb' found"

<OAI-PMH xmlns="http://www.openarchives.org/OAI/2.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.openarchives.org/OAI/2.0/ http://www.openarchives.org/OAI/2.0/OAI-PMH.xsd">
  <responseDate>2024-01-04T21:29:01Z</responseDate>
  <request>http://localhost:8080/oai</request>
  <error code="badArgument">No argument 'verb' found</error>
</OAI-PMH>

/oai?verb=foo&verb=bar will show "Verb must be singular, given: '[foo, bar]'"

<OAI-PMH xmlns="http://www.openarchives.org/OAI/2.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.openarchives.org/OAI/2.0/ http://www.openarchives.org/OAI/2.0/OAI-PMH.xsd">
  <responseDate>2024-01-04T21:29:01Z</responseDate>
  <request>http://localhost:8080/oai</request>
  <error code="badArgument">Verb must be singular, given: '[foo, bar]'</error>
</OAI-PMH>

Which issue(s) this PR closes:

• Closes Malformed OAI-PMH requests generate an internal server error #9275

Special notes for your reviewer:

In Slack I checked with @landreev and he's fine with the 200 response. "You have successfully queried the OAI server, and here’s your answer" even though the answer is "there was an error." This follows an established pattern set by (at least) "no set found", which I added a test for.

Due to how the xoai library is written, I have to return <error code="badArgument"> rather than ` as the spec requires. ( http://www.openarchives.org/OAI/openarchivesprotocol.html#ErrorConditions says "Value of the verb argument is not a legal OAI-PMH verb, the verb argument is missing, or the verb argument is repeated."). However, Leonid and I are ok with deferring this as it requires work on the library side.

I updated the docker compose file so I could test this.

Related:

• OAI-PMH responses indicating errors should be processable by OAI-PMH clients #3797

Suggestions on how to test this:

Play around with variations on these:

• /oai?foo=bar will show "No argument 'verb' found"
• /oai?verb=foo&verb=bar will show "Verb must be singular, given: '[foo, bar]'"

Confirm I didn't break anything (HarvestingServerIT passes for me locally.)

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

No.

Is there a release notes update needed for this change?:

Yes, included.

Additional documentation:

None.

[coveralls]

coverage: 20.152% (-0.001%) from 20.153%
when pulling edd6fc8 on 9275-harvest-invalid-query-params
into e5e232d on develop.

[landreev]

Perfect!

[pdurbin]

@landreev from https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-10205/3/testReport/ we're seeing this:

Any insight on this? I could swear I've seen this switch from noRecordsMatch to noSetHierarchy and back again, but that doesn't make sense. It should be one or the other, right?

[pdurbin]

I just merged develop to kick off another Jenkins run to get another data point. I'm not sure if the error is flipping back and forth or not. Let's see.

[pdurbin]

In 30e357b I changed the "no such set" test to expect noSetHierarchy rather than noRecordsMatch.

[landreev]

If the set does not exist, then I would expect it to consistently return "no set hierarchy". I can't really think of a timing issue or anything like that, that would be causing a "no records match" intermittently.

[landreev]

Wait, I was clearly wrong about the above.
You only get the "no set hierarchy" when there are NO sets on this server, neither the requested set, nor any others.
So yes, the response will be different, depending on what else the developer has in their database/whether other tests have already run.

You know, I would go as far as to suggest to drop this test from the PR. Because
a) it's a bit out of scope - since the issue was specifically about a 500 for a bad verb request
b) I have another PR currently in review/QA, #10228 that changes the behavior of ListSets slightly that may actually conflict with this.

[landreev]

... I should've said, I have 2 PRs in the works, #10222 and #10228 that change the behavior of both ListIdentifiers and ListSets, that may conflict with this test.

[landreev]

... Although feel free to just modify the test, to expect either of the 2 error codes, "no sets" or "no records matching"; up to you.
Also, should we pick a weirder set name than "census" - in case a developer running the tests happens to have a set named that? 😄

[landreev]

(sorry for the confusion; too many OAI PRs going on in parallel...)

[pdurbin]

So yes, the response will be different, depending on what else the developer has in their database/whether other tests have already run.

That's enough to convince me to simply drop the test, which I did in edd6fc8.

Thanks for all the feedback, @landreev

[github-actions]

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:9275-harvest-invalid-query-params

ghcr.io/gdcc/configbaker:9275-harvest-invalid-query-params

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

[pdurbin]

Tests are passing: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-10205/7/testReport/edu.harvard.iq.dataverse.api/

@landreev there's probably no need for you to be assigned to this PR. Thanks for the feedback. I'll remove you.