Skip to content

fix(query): don't swallow cursor:eval errors as HTTP 200 'Invalid context-item' (regression in 0.9.7) #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joewiz wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix(query): don't swallow cursor:eval errors as HTTP 200 'Invalid context-item' (regression in 0.9.7) #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion modules/api.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,14 +428,42 @@
}
},
"400": {
"description": "Bad request — invalid or missing query",
"description": "Bad request — missing `query` field or malformed `context-item` XML",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/Error"
}
}
}
},
"404": {
"description": "`context-path` was supplied but the document at that path doesn't exist",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/Error"
}
}
}
},
"500": {
"description": "Server-side error during query compilation or evaluation. Body carries the structured XPathException fields (`code`, `description`, `line`, `column`, `module`, `value`) so clients can surface the real cause.",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"code": { "type": "string", "description": "Error QName, e.g. `err:XPST0081`" },
"description": { "type": "string" },
"line": { "type": "integer" },
"column": { "type": "integer" },
"module": { "type": "string", "nullable": true },
"value": { "nullable": true }
}
}
}
}
}
}
}
Expand Down
69 changes: 44 additions & 25 deletions modules/query.xqm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ xquery version "3.1";
module namespace query="http://exist-db.org/api/query";

import module namespace cursor="http://exist-db.org/xquery/cursor";
import module namespace roaster="http://e-editiones.org/roaster";

declare namespace output="http://www.w3.org/2010/xslt-xquery-serialization";

Expand Down Expand Up @@ -59,36 +60,54 @@ declare function query:execute($request as map(*)) {
return
if (empty($expression) or $expression = "")
then
map { "error": "Missing required field: query" }
roaster:response(400, "application/json",
map { "error": "Missing required field: query" })
else
(: context-path (a DB path the editor has open) takes precedence
: over context-item (inline serialized XML). Either resolves to
: the node `expression` will see as `.` / the focus of
: unprefixed path expressions like `//foo`. :)
try {
let $context-item :=
if (exists($context-path) and $context-path != "") then
if (doc-available($context-path))
then doc($context-path)
else error(xs:QName("query:context-path-not-found"),
"context-path not found: " || $context-path)
else if (exists($context-item-xml) and $context-item-xml != "") then
parse-xml($context-item-xml)
else
()
let $mlp := if ($module-load-path) then $module-load-path else ()
return
if (exists($context-item)) then
cursor:eval($expression, $mlp, $context-item)
else if (exists($mlp)) then
cursor:eval($expression, $mlp)
else
cursor:eval($expression)
} catch query:context-path-not-found {
map { "error": $err:description }
} catch * {
map { "error": "Invalid context-item: " || $err:description }
}
let $context-item := query:resolve-context-item($context-path, $context-item-xml)
return
if ($context-item instance of map(*) and exists($context-item?error))
then
roaster:response($context-item?status, "application/json",
map { "error": $context-item?error })
else
let $mlp := if ($module-load-path) then $module-load-path else ()
return
if (exists($context-item)) then
cursor:eval($expression, $mlp, $context-item)
else if (exists($mlp)) then
cursor:eval($expression, $mlp)
else
cursor:eval($expression)
};

(:~
: Resolve the optional context item for query:execute. Returns a node()
: on success, an error map { status, error } if context-path is missing
: or context-item is malformed XML, or empty-sequence if neither is
: supplied. The error map is wrapped here rather than propagating an
: XPath error so cursor:eval's own errors (the user's XQuery has bugs)
: pass through unmodified and Roaster gives them a proper HTTP status.
:)
declare %private function query:resolve-context-item(
$context-path as xs:string?,
$context-item-xml as xs:string?
) {
if (exists($context-path) and $context-path != "") then
if (doc-available($context-path))
then doc($context-path)
else map { "status": 404, "error": "context-path not found: " || $context-path }
else if (exists($context-item-xml) and $context-item-xml != "") then
try {
parse-xml($context-item-xml)
} catch * {
map { "status": 400, "error": "Invalid context-item: " || $err:description }
}
else
()
};

(:~
Expand Down
51 changes: 47 additions & 4 deletions src/test/cypress/e2e/query.cy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,16 +238,55 @@ describe('/api/query', () => {
});

describe('error handling', () => {
it('returns error for missing query', () => {
it('returns HTTP 400 for missing query', () => {
cy.request({
url: '/api/query',
method: 'POST',
auth,
body: { query: '' }
body: { query: '' },
failOnStatusCode: false
}).then(response => {
expect(response.status).to.eq(400);
expect(response.body).to.have.property('error');
});
});

// Regression: PR #41's try/catch wrapped every cursor:eval error with
// "Invalid context-item:" AND returned HTTP 200, breaking eXide's
// error-display path (which relies on `if (!response.ok)`).
// cursor:eval errors must now propagate with their original code/
// description and a non-200 status.
it('lets cursor:eval errors propagate with structured fields and HTTP 5xx', () => {
cy.request({
url: '/api/query',
method: 'POST',
auth,
body: { query: '//does-not-exist:foo' },
failOnStatusCode: false
}).then(response => {
expect(response.status).to.be.oneOf([400, 500]);
// Structured XPathException → JSON: { code, description, line, column, module, value }
expect(response.body).to.have.property('code');
expect(response.body.code).to.match(/XPST0081/);
expect(response.body).to.have.property('description');
expect(response.body).to.have.property('line');
// Must NOT be mislabeled as a context-item error
expect(response.body.description).to.not.include('Invalid context-item');
});
});

it('returns HTTP 400 with proper prefix for malformed context-item', () => {
cy.request({
url: '/api/query',
method: 'POST',
auth,
body: { query: '.', 'context-item': '<not-well-formed' },
failOnStatusCode: false
}).then(response => {
expect(response.status).to.eq(400);
expect(response.body.error).to.include('Invalid context-item');
});
});
});

describe('documentURI / nodeId in results (issue #40)', () => {
Expand Down Expand Up @@ -402,8 +441,10 @@ describe('/api/query', () => {
url: '/api/query',
method: 'POST',
auth,
body: { query: '.', 'context-item': '<not-well-formed' }
body: { query: '.', 'context-item': '<not-well-formed' },
failOnStatusCode: false
}).then(response => {
expect(response.status).to.eq(400);
expect(response.body).to.have.property('error');
expect(response.body.error).to.include('context-item');
});
Expand All @@ -414,8 +455,10 @@ describe('/api/query', () => {
url: '/api/query',
method: 'POST',
auth,
body: { query: '.', 'context-path': '/db/nonexistent-xyz.xml' }
body: { query: '.', 'context-path': '/db/nonexistent-xyz.xml' },
failOnStatusCode: false
}).then(response => {
expect(response.status).to.eq(404);
expect(response.body).to.have.property('error');
expect(response.body.error).to.include('not found');
});
Expand Down