fix(list,i18n): a 400 from the server no longer reads as "check your connection" - #3066
Merged
Conversation
…connection" classifyLoadError exists because a 403 rendered the same "check your connection and try again" panel as a genuine outage — its own doc comment says users "were told to debug their network when the server had (correctly) denied them access." It made that distinction for 401 and 403, then sent everything else, 4xx included, to the network branch. A 400 is the server saying it understood the request and will never accept it. Retrying resends the identical bad request, so the connection copy is advice that cannot work — the same mistake the function exists to prevent, one status code over. Reachable from ordinary stored metadata since objectstack#4121: a `$filter` array that is not a filter AST is now rejected at the protocol with 400 INVALID_FILTER, where it previously reached a driver (and, for a lone ['and'], silently returned every row). A view saved with such a filter now answers 400 on every load. Adds a fourth classification, `rejected`, for status 400 and the server's 400-class codes (INVALID_FILTER, UNSUPPORTED_QUERY_PARAM, INVALID_QUERY). Its copy points at the filter rather than the network and says who can fix it when the view is saved that way. 403/401 keep priority so a permission denial can never read as a bad request — pinned by a test. The two strings go into ALL TEN locale packs, not just en: the neighbouring panels are translated and fallbackLng would have rendered this one in English beside them. all-locales-key-parity.test.ts caught the pack I missed. 5 new tests (numeric status, code without status, status in message text, 403/401 ordering); reverting the branch fails four. plugin-list + i18n: 403 tests / 29 files green. Refs objectstack-ai/objectstack#4121, #2945 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…ther the query expands a lookup (#3084) #3072 single-sourced the ARRAY branch of the adapter's two `find()` routes. The object branch was left as it was: `convertQueryParams` converted a MongoDB-style filter to AST while `translateFilterToAST` returned it verbatim — so the same `$filter` went out in two formats, decided by whether the query happened to expand a lookup. Measured across 21 operator shapes, four diverged. Most of the gap turned out to be harmless, which is worth recording because it was not obvious: `{$and: […]}` survives the plain route as a `['$and','=',[…]]` comparison that `parseFilterAST` reads back as a real `$and`, and `$exists` vs `$null` is a difference the server treats identically. Two were not harmless: - THE UNKNOWN-OPERATOR GUARD ONLY RAN ON ONE ROUTE. `convertFiltersToAST` throws on an unrecognised operator, with a comment saying it does so "to avoid silent failure" — but the expanded route never called it, so a typo'd operator threw on a plain read and shipped silently whenever a lookup was expanded. - `$regex` WAS SILENTLY REWRITTEN TO `contains`. The existing test's own example makes the case: `$regex: '^John'` means "starts with John", while `contains '^John'` looks for a literal caret — so "John Smith" does not match. A different question, not a weaker version of the same one, and neither result looks wrong on screen. The rewrite sat behind a `console.warn`, which is not an error channel in a deployed app, and the function's own unknown-operator message never listed `$regex` among the supported set. The spec has no `$regex` (`FILTER_OPERATORS`, data/filter.zod.ts), so there is nothing to translate it into: it is refused now, the same treatment the neighbouring unknown operator already got. Nothing in the repo depended on the conversion. Both refusals throw `FilterOperatorError` carrying `code: 'INVALID_FILTER'` / `httpStatus: 400`. The pre-existing unknown-operator throw was a bare `Error`, which `classifyLoadError` reads as a network fault — so a malformed filter told the user to check their connection (#3066), the one thing it was not. `filter-converter.test.ts`'s `$regex` case asserted the old behaviour and is rewritten to assert the refusal, keeping the `'^John'` example because it demonstrates the harm better than any prose. Verification: 9 new/changed tests; reverting the two source files fails 9 of them. Full suite 762 files / 8875 tests green; tsc clean; eslint 0 errors. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
The one objectui follow-up needed for this session's framework changes. Found by auditing each merged framework PR against objectui rather than assuming.
The defect
classifyLoadError(ListView.tsx) exists because a 403 used to render the same "check your connection and try again" panel as a genuine outage. Its own doc comment:It made that distinction for 401 and 403, then sent everything else — 4xx included — to the
networkbranch.A 400 is the server saying it understood the request and will never accept it. Retrying resends the identical bad request, so the connection copy is advice that cannot work. Same mistake, one status code over.
Why now
objectstack-ai/objectstack#4121 made this reachable from ordinary stored metadata: a
$filterarray that is not a filter AST is now rejected at the protocol with400 INVALID_FILTER. Previously it reached a driver — and for a lone['and']it silently returned every row.So a view saved with such a filter now answers 400 on every load, and objectui was telling the user to check their network. The server change is right; this is the client half.
The change
A fourth classification,
rejected, forstatus === 400and for the server's 400-class codes (INVALID_FILTER,UNSUPPORTED_QUERY_PARAM,INVALID_QUERY). Its copy points at the filter rather than the network, and says who can fix it when the view is saved that way:403/401 keep priority, so a permission denial can never read as a bad request. That ordering is pinned by a test rather than left to the reading order of the
ifs.All ten locale packs, not just
enThe neighbouring panels are translated, and
fallbackLng: 'en'would have rendered this one in English beside them — visible only to a non-English user, which is how the two failure modesall-locales-key-parity.test.tsdescribes stay invisible.That gate caught a pack I missed: my first pass added nine, because the
greplisting them was truncated by ahead. The test namedfrand the exact two keys.Verification
httpStatus, an errorcodewith no numeric status, a status embedded in the message text ("HTTP 400 Bad Request — {}"), and a 403/401 ordering guard.plugin-list+i18n: 403 tests across 29 files, green.eslint: 0 errors.What else was checked, and needed nothing
combo,WidgetActionType=ActionTypecombo; the widget enum widening is what unblocksform, which its dispatcher already implementssaveMetapersists canonical operatorsDatasetWidgetalready renders a failed query as an error panel, so the new throws surface as a visible message instead of a wrong numberInlineActionSchemaElementButtonrenderer still forwardsactionType/endpoint/navigate/description, which the schema does not declare; trimming them is a narrowing that belongs with the Stage C alias removalRefs objectstack-ai/objectstack#4121, #2945
🤖 Generated with Claude Code