Skip to content

Say why a call failed, and stop padding the ones that succeed - #9

Open
Robbie1977 wants to merge 1 commit into
mainfrom
fix/error-bodies-and-payload-size
Open

Say why a call failed, and stop padding the ones that succeed#9
Robbie1977 wants to merge 1 commit into
mainfrom
fix/error-bodies-and-payload-size

Conversation

@Robbie1977

Copy link
Copy Markdown

Four defects, all found by running every tool against the live service after the v1.10.0 deploy. Ships as v1.11.0.

1. The error body was being thrown away

Ten catch sites interpolated the caught exception directly (${error}). A caller who passed an unknown query_type got this back:

Error running query "NotARealQueryType" on "FBbt_00003686": AxiosError: Request failed with status code 400

The reason was sitting inside that exception the whole time. The same request via curl returns a 400 whose body is {"error": "Unknown query_type: NotARealQueryType", "available": [...43 names...]}. Now:

Error running query "NotARealQueryType" on "FBbt_00003686": Unknown query_type: NotARealQueryType.
Valid values: AlignedDatasets, AllAlignedImages, AllDatasets, AnatomyExpressedIn, ... scRNAdatasetData. (HTTP 400)

rejectionDetail was already written but wired into only two of the ten sites. It now also:

  • Handles a timeout (ECONNABORTED/ETIMEDOUT), which has no body at all and is the failure most likely to be misread as "there is no data". The client gives up at 60s but the server keeps computing against its 180s budget and caches the result, so the honest advice is to retry — and that is what the message now says.
  • Appends the server’s available and suggestions arrays, so a rejection carries the alternatives instead of forcing another guess.
  • Notes a status: "computing" body (the 503 the origin returns on a budget overrun).

A new failureText(context, error) puts every catch site on the same path and appends the HTTP status.

2. fetchAvailableQueryTypes returned objects where names were expected

Declared Promise<string[] | null>, but it returned result.data.Queries raw — and each entry there is an object ({query, label, function, takes, preview, preview_columns, preview_results, output_format, count}). formatAvailableQueriesHint then JSON.stringify’d the lot, so the error path — the path where the caller is already confused — emitted every query’s argument schema and empty preview block. It now maps to q.query and joins with commas.

3. get_hierarchy returned the same tree three times

Once as data in descendants/ancestors, once as a display string, once as a byte-identical display_full, and once as an html document written for the VFB site’s ROI browser. On a one-level Kenyon cell tree the HTML alone was 3.2 KB of a 4.3 KB response.

New include_html parameter, default false. display_full is dropped when it duplicates display. 4.3 KB → 1.4 KB. The response carries a trimmed field saying what went and how to get it back.

4. get_term_info was mostly scaffolding

Every entry in Queries carried its argument schema plus a preview_results block whose headers repeated preview_columns verbatim above an empty rows — seven times over on FBbt_00100249. Every image listed six file URLs differing only in filename.

Trimmed by default: 15.7 KB → 7.6 KB (52% smaller). Nothing is unrecoverable — the trimmed field names the dropped file URLs (they sit beside the thumbnail as thumbnailT.png, volume.nrrd, volume.wlz, volume_man.obj, volume.swc), and verbose: true returns the untouched response.

Testing

test/live-smoke.js (npm run test:live) drives the built server over stdio and checks all four against the live service — 14 assertions, all passing. Happy-path regression on list_connectome_datasets, search_terms, resolve_combination and list_search_facets confirms the failureText rewiring did not disturb the success paths.

Calls are issued one at a time with 7–8s between them, and every one goes through v3-cached, per the standing rule about load on VFBquery.

Review notes

  • The two new parameters both default to the slimmer behaviour, so an existing client that sends neither sees a smaller payload than before. That is the point, but it is a behaviour change for anyone parsing html or Queries[].takes — hence the minor bump to 1.11.0 rather than a patch.
  • search_termsauto_fetch_term_info path folds get_term_info into its response; that call now gets the trimmed shape too. Intentional.
  • Version bumped in package.json, package-lock.json and server.json.

Four defects found by testing every tool against the live service.

An error body was thrown away. Ten catch sites interpolated the caught
exception directly, so a caller who passed an unknown query_type got
"AxiosError: Request failed with status code 400" while the response body
sitting in that exception said "Unknown query_type: NotARealQueryType" and
listed all 43 names it would have accepted. rejectionDetail now also handles
a timeout (the server keeps computing and caches the result, so retrying is
the right advice, and that is the failure most likely to be misread as "no
data"), appends the server's `available` and `suggestions` lists, and notes
a `status: computing` body. A new failureText() puts every catch site on it.

fetchAvailableQueryTypes was typed Promise<string[]> but returned the raw
Queries array, whose entries are objects. formatAvailableQueriesHint then
JSON.stringify'd them, so the run_query error path emitted every query's
argument schema and empty preview block. It now maps to the query names.

get_hierarchy returned the same tree three times: as data, as `display`, as
a byte-identical `display_full`, and as an `html` document written for the
VFB site's ROI browser. The HTML is now behind include_html (default false)
and display_full is dropped when it duplicates display. 4.3 KB -> 1.4 KB on
a one-level Kenyon cell tree.

get_term_info carried an argument schema and an empty preview block for
every entry in Queries, and six file URLs for every image. Trimmed by
default with a `trimmed` field naming what went and where the dropped file
URLs live, so nothing is unrecoverable; verbose: true returns the untouched
response. 15.7 KB -> 7.6 KB on FBbt_00100249.

test/live-smoke.js drives the built server over stdio and checks all four
against the live service; npm run test:live. Calls are spaced, and every one
goes through v3-cached.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8ZAhRM7gh8arDx9SuSWcT
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