Give a descriptive error when a DOI cannot be resolved to citation metadata - #1900
Open
adityasingh2400 wants to merge 1 commit into
Open
Give a descriptive error when a DOI cannot be resolved to citation metadata#1900adityasingh2400 wants to merge 1 commit into
adityasingh2400 wants to merge 1 commit into
Conversation
…n metadata `dandi service-scripts update-dandiset-from-doi` crashed with a bare `json.JSONDecodeError: Expecting value: line 1 column 1 (char 0)` traceback whenever doi.org answered with anything other than JSON. Two things were wrong. The CSL Accept header was set on the `RESTFullAPIClient` session, but `RESTFullAPIClient.request()` sets `accept: application/json` on the request itself whenever `json_resp` is true, and per-request headers win over session headers. The resolver therefore never saw the citation format we meant to ask for. The checked-in VCR cassettes record this: every captured request carries `accept: application/json`. Crossref happens to serve JSON for that, which is why the existing tests pass, but a resolver that does not will redirect to the landing page and return HTML with a 200. And when that happened there was no error handling at all, so the user got a `JSONDecodeError` out of the requests internals with nothing pointing at the DOI. The fetch now moves into `fetch_doi_citation_metadata()`, which requests the raw response so the intended CSL Accept header survives, and turns a 404, another HTTP error, a non-JSON body, and a non-object body each into a `click.ClickException` naming the DOI, the URL, and the content type received. `normalize_doi()` additionally accepts the DOI as a bare DOI, a `doi:` URI, or a resolver URL, and rejects anything else with a `click.UsageError` instead of a traceback. That also fixes the `relatedResource` record, whose url was built as `https://doi.org/{doi}` and so came out doubled when the user passed a resolver URL. The lookup now happens before connecting to the archive, so a bad DOI fails fast without needing credentials. Closes dandi#1855
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1855
update-dandiset-from-doicrashed with a barejson.JSONDecodeError: Expecting value: line 1 column 1 (char 0)whenever doi.org answered with anything other than JSON.There are two separate problems behind that traceback.
The first is a header bug. The CSL Accept header was set on the
RESTFullAPIClientsession, butRESTFullAPIClient.request()setsaccept: application/jsonon the request wheneverjson_respis true, and in requests a per-request header wins over a session header. The resolver therefore never saw the citation format we meant to ask for. The checked-in VCR cassettes record this, every captured request carriesaccept: application/jsonand never the CSL type. Crossref happens to serve JSON for that anyway, because it redirects toapi.crossref.org/.../transform, which is why the existing tests pass. A registration agency that does not will redirect to the landing page and return HTML with a 200, which is the reported failure on a DataCite10.48324DOI.The second is that there was no error handling at all on that path, so the user got a
JSONDecodeErrorraised from inside requests with nothing naming the DOI.The fetch moves into
fetch_doi_citation_metadata(), which requests the raw response so the intended CSL Accept header survives, and turns a 404, any other HTTP error, a non-JSON body, and a non-object body each into aclick.ClickExceptionnaming the DOI, the URL, and the content type actually received.normalize_doi()now accepts a bare DOI, adoi:URI, or a resolver URL, and rejects anything else with aclick.UsageErrorinstead of a traceback. That also fixes therelatedResourcerecord, whose url was built ashttps://doi.org/{doi}and came out doubled when the user passed a resolver URL. The lookup now happens before connecting to the archive, so a bad DOI fails fast without needing credentials.Only
title,abstract, andauthor[*].given/family/ORCID/affiliationare read, and all of those are present in both CSL JSON and the Crossref record the cassettes captured, so replaying the existing cassettes is unaffected. vcrpy matches on method and URI, not headers.Verified against the base ref with doi.org mocked to serve HTML at 200. Before, the run ends in
requests.exceptions.JSONDecodeErrorfromdandiapi.pyline 326. After, it reports that the DOI answered withtext/htmlinstead of CSL JSON and explains that the registration agency likely does not serve citation metadata. The Accept header actually sent went fromapplication/jsontoapplication/vnd.citationstyles.csl+json; charset=utf-8.New tests are marked
@pytest.mark.ai_generated. They give 18 passed. The 6 deselected are the VCRtest_update_dandiset_from_doicases, which need the docker archive fixture and could not run locally.AI assistance disclosure: this change was written with the help of Claude Code, and the added tests are marked
ai_generatedas CLAUDE.md asks. I reviewed and tested everything before submitting.