Skip to content

Add try/catch around cql call#758

Merged
danamansana merged 2 commits into
mainfrom
NOREF/better-aggregations-cql-error
Jun 11, 2026
Merged

Add try/catch around cql call#758
danamansana merged 2 commits into
mainfrom
NOREF/better-aggregations-cql-error

Conversation

@danamansana

Copy link
Copy Markdown
Contributor

Addresses an error found in today's alarms: when a patron searches for an invalid CQL query, the RC searches both /resources and /resources/aggregations. We are correctly responding with a 422 for the first path, but the second path responds with a 500 error that is invisible to the patron but clutters our logs.
This PR adds a try/catch block around the parsing so that we can respond with a 422 in this case and not log an error.
Future work in RC may investigate whether it is possible to avoid this call altogether.

Comment thread lib/resources.js
const query = (cqlQuery || new CqlQuery(params.q)).buildEsQuery(request)
return query
} catch (e) {
throw new InvalidQuerySyntaxError(e.message)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be a logger.warn, or else it will continue to trigger alarms.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error is caught higher up and converted to a 422 response. Running it locally I don't see any errors in the logs

@charmingduchess

Copy link
Copy Markdown
Contributor

for posterity, this is the ticket cut for this alarm
https://newyorkpubliclibrary.atlassian.net/browse/SCC-5421

@danamansana danamansana merged commit b64cdee into main Jun 11, 2026
4 checks passed
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.

3 participants