fix(query): don't swallow cursor:eval errors as HTTP 200 'Invalid context-item' (regression in 0.9.7)#46
Open
joewiz wants to merge 1 commit into
Open
Conversation
…text-item' Regression introduced in eXist-db#41 (context-item support): the new try/catch caught EVERY error from query:execute and rewrapped it as HTTP 200 { error: "Invalid context-item: <original cause>" } That broke two downstream contracts at once: - The original cause was mislabeled. A simple XPST0081 namespace error now read "Invalid context-item: ... No namespace defined for prefix does-not-exist:foo", which is wrong — the context item was fine; the user's XQuery referenced an undeclared prefix. - The HTTP status was 200. eXide's #run handler relies on \`if (!response.ok)\` to detect query errors and surface the pill + panel; with 200, it thinks the query succeeded and tries to read data.cursor (undefined). Caught by running eXide's query_error_display / query_error_structured cypress specs against eXist 7 beta3. Fix: - Move the parse-xml() call into its own try/catch inside a new query:resolve-context-item helper, so its failure is the only thing that gets the "Invalid context-item:" prefix. - Resolution returns an { status, error } map for the two error modes (404 for missing context-path, 400 for malformed context-item-xml); the caller wraps that in roaster:response(status, ...) so the HTTP code matches. - cursor:eval errors are no longer caught at all — they propagate to Roaster, which renders the standard XPathException → JSON shape ({ code, description, line, column, module, value }) and a 500 status. That's what eXide's evalError handler already expects (see src/eXide.js:937). api.json updated to declare the 400, 404, and 500 responses so Roaster doesn't fall back to XML serialization on them. Cypress: existing context-item error tests updated to assert the new HTTP status codes; one new regression test pins the cursor:eval-error path to "HTTP 5xx + structured XPathException + no Invalid-context-item prefix". Verified against eXide develop HEAD: all 36 cypress specs pass (260/262, with 2 intentional skips), up from 258/262 before the fix. The two specs that were failing — query_error_display and query_error_structured — both pass now.
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.
[This PR was co-authored with Claude Code. -Joe]
Regression fix for
0.9.7. Spotted while running eXide develop HEAD's cypress suite against eXist 7 beta3 + existdb-openapi 0.9.7: two specs (query_error_display_spec,query_error_structured_spec) failed because the existdb-openapi/api/queryendpoint was swallowing every error fromcursor:evalasHTTP 200 { error: "Invalid context-item: …" }.Root cause
The try/catch I added in #41 (context-item support) was too broad:
A user's
//does-not-exist:foo(anXPST0081namespace error fromcursor:eval) came back as:Two contracts broken at once:
#runhandler (src/eXide.js:937) detects query errors viaif (!response.ok). HTTP 200 → it thinks the query succeeded, tries to readdata.cursor(undefined), and the error pill / panel never lights up. That's the symptom the eXide cypress specs were catching.Fix
query:resolve-context-item. That's the only place the"Invalid context-item:"prefix is now applied.{status, error}map for the two failure modes — 404 for a missingcontext-path, 400 for malformedcontext-itemXML — and the caller wraps it inroaster:response(status, ...)so the HTTP code matches.cursor:evalerrors are no longer caught. They propagate to Roaster, which renders the standardXPathException → JSONshape ({code, description, line, column, module, value}) with an HTTP 500. That's exactly what eXide'seditor.evalErrorhandler already expects.modules/api.jsonupdated to declare 400, 404, and 500 responses so Roaster doesn't fall back to XML serialization on them.After the fix
Verified
query_error_display_spec,query_error_structured_spec) both pass now.Apologies
This was a regression I shipped in 0.9.7. Cutting a 0.9.8 once this lands.