Skip to content

Hardcover search never returns any results - #71

Open
JordanFromIT wants to merge 1 commit into
Chaptarr:developfrom
JordanFromIT:fix/hardcover-graphql-batching
Open

Hardcover search never returns any results#71
JordanFromIT wants to merge 1 commit into
Chaptarr:developfrom
JordanFromIT:fix/hardcover-graphql-batching

Conversation

@JordanFromIT

Copy link
Copy Markdown

Description

Hardcover search never returns any results. Chaptarr packs its three lookups — Author, Book and Series — into a single HTTP request as a JSON array of GraphQL operations, and Hardcover rejects an array body outright with 400 invalid_query. The same thing happens on the author-books follow-up, which batches its id chunks the same way. Because the failure is at the transport layer rather than the query layer, it happens for every user with every valid token, and the surfaced error points somewhere else entirely — mine said "Hardcover's API is currently unavailable. Please try again later." while Hardcover was perfectly healthy. This sends each operation as its own request. The queries, the variables and all of the response handling are unchanged.

Reproduced against the live API with a valid token, changing nothing but the shape of the body:

Request body Result
[{query,variables}, {…}, {…}] — what Chaptarr sends 400 {"error":"invalid_query","error_description":"Unexpected end of document"}
{query,variables} — one operation 200, 817 hits

That is the whole bug. The GraphQL itself is fine — Chaptarr's exact search query, and its exact token-validation query, both return 200 when sent on their own.

Technical detail

Two call sites build an array body:

  • ExecuteSearchSearchTargets.Select(...).ToList() serialised straight into the request, so all three search types travel together.
  • The author-books lookup — authorIdChunks.Select(...).ToList(), one operation per chunk of five author ids.

Both now loop and send one operation per request. To keep that from duplicating request setup a third time, the builder is extracted into BuildGraphQlRequest(jsonContent), which is the same construction as before including adding Authorization after Build().

Response handling follows the body shape. TryExtractSearchPayloads (which required a top-level array whose length matched SearchTargets.Length) is replaced by a TryExtractSearchPayload(queryType, responseContent, …) overload that parses one response and delegates to the existing per-operation logic. That per-operation method — including its GraphQL errors handling and the UNAUTHENTICATED detection — is untouched. The author-books loop likewise now reads data from each response directly instead of indexing into root[chunkIndex].

ENRICHMENT_QUERY already sent a single operation and is not affected.

Deliberate choices worth flagging for review:

  • This trades one request for three (plus one per author-books chunk, as before, but now unbatched). That is the honest cost and the thing most worth challenging here. Batching existed to be frugal, and I have not tried to preserve it — because there is no batch form Hardcover currently accepts, so the choice is three requests or zero results. If you would rather gate this behind a capability probe, or serialise them with a small delay to be gentler on rate limits, say so and I will rework it.
  • Failure semantics are unchanged in kind, but earlier in time. A failing operation still abandons the whole search rather than merging partial results — the existing comment "Do not silently merge partial operation data with successful result types" still holds. The difference is that a first-operation failure now short-circuits before the remaining types are requested, which is strictly less work.
  • Existing tests asserted the broken behaviour. should_send_one_search_root_per_http_batched_operation explicitly asserted ValueKind == Array with three operations, and should_chunk_ten_author_book_roots_into_two_http_batched_operations asserted a two-element array body. Both are rewritten to assert the per-request shape and renamed accordingly. I have kept every assertion that was about content and changed only the ones about packaging, so the coverage those tests provided is not lost.
  • No capability detection. I did not add a fallback that tries a batch first and retries unbatched, because a 400 on every search is not a state worth optimising for, and the retry would double the failure latency for anyone Hardcover has already stopped accepting batches from.

Known gap, deliberately not addressed: ValidateHardcoverToken maps any unrecognised status to "Hardcover's API is currently unavailable", which is what sent me looking at Hardcover's status page instead of at this. Improving that message is a separate, unrelated change and I did not want to bundle it. Happy to follow up.

Database Migration

NO. No schema changes, no new columns, no stored data of any kind. The change affects only how an outbound HTTP request is framed; nothing is persisted, so there is nothing to migrate or back-fill and reverting the commit fully reverts the behaviour.

How was this tested?

Built and tested on Linux, .NET 10, in the mcr.microsoft.com/dotnet/sdk:10.0 image, using the same steps build.yml runs:

Step Result
git grep merge-conflict markers clean
package.json parses ok
version_guard.py sync Version sync OK for working tree: 0.9.929
version_guard.py monotonic --compare-ref origin/develop Version monotonic OK: 0.9.929
version_guard.py commit-hygiene --compare-ref origin/develop OK for range origin/develop..HEAD
dotnet build src/Chaptarr.NoTests.sln -c Release 0 warnings, 0 errors
dotnet test src/Chaptarr.Core.Test -c Release 2839 passed, 0 failed

Test counts. Clean origin/develop at 5713d83: 2838 passed. This branch: 2839 passed, 0 failing. Both measured from a hard-reset, git cleaned tree with _output/_tests removed. HardcoverSearchClientFixture itself goes from 42 to 43 tests.

No frontend files are touched — 2 files changed, both .cs — so the yarn steps outside build.yml are not applicable.

New test: should_send_each_search_type_as_its_own_request asserts three separate requests, each with a top-level JSON object body, carrying the three distinct query_type values and the same q / limit / page variables as before.

Confirmed the tests catch the bug by restoring only HardcoverSearchClient.cs to its develop version while keeping the new tests, then re-running the fixture:

Failed should_send_each_search_type_as_its_own_request
Failed should_send_exactly_one_search_root_per_request
Failed should_chunk_ten_author_book_roots_into_two_separate_requests
Failed should_merge_live_shaped_author_book_and_series_results
Failed should_retry_a_server_error_once_then_continue_the_search
Failed should_anchor_an_exact_author_then_place_provider_id_books_and_series_below_them
Failed should_anchor_the_closest_book_then_place_its_provider_id_author_and_series_below_it
Failed should_use_hardcover_text_match_to_choose_between_non_exact_entity_types
Failed!  - Failed: 8, Passed: 35, Total: 43

Restoring the fix returns Passed! - Failed: 0, Passed: 43.

End to end, Docker on Linux, against a real instance with a valid Hardcover token. Before this change every Hardcover search failed with 400 BadRequest, logged as Hardcover search failed. After:

Call Before After
GET /api/v1/search?term=Harry Potter and the Philosopher's Stone 503, Hardcover search failed 200, 21 results — correct book, author and series all present
GET /api/v1/series/lookup?foreignSeriesId=… 400 200Harry Potter, workCount 7, primaryWorkCount 7, books returned in positions 1–7

Both search and the series lookup path exercise the batching code, and both were dead before the change.

Screenshots (UI changes only)

None — no UI changes.

Hardcover rejects a batched request body — a JSON array of operations — with
400 invalid_query ("Unexpected end of document"). Search sent all three of its
lookups that way, and the author-books follow-up sent its id chunks the same
way, so Hardcover search fails for every user regardless of their token.

Send each operation as its own request instead. The query text, variables and
response handling are unchanged; only the transport shape differs.
@robertlordhood

Copy link
Copy Markdown
Contributor

The errors you were seeing were part of hardcover switching things on their end. Are you still seeing issues now they've finished the migration? You shouldn't be, let me know

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.

2 participants