Skip to content

fix(schema-compiler): Throw UserError for unknown granularity so API returns 400 instead of 500#11288

Merged
ovr merged 1 commit into
cube-js:masterfrom
BrianSigafoos-SQ:fix/unknown-granularity-user-error
Jul 20, 2026
Merged

fix(schema-compiler): Throw UserError for unknown granularity so API returns 400 instead of 500#11288
ovr merged 1 commit into
cube-js:masterfrom
BrianSigafoos-SQ:fix/unknown-granularity-user-error

Conversation

@BrianSigafoos-SQ

Copy link
Copy Markdown
Contributor

Check List

  • Tests have been run in packages where changes have been made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Description of Changes Made

Querying a time dimension with an unknown granularity (for example granularity: "all", which some older clients send) throws a plain Error from Granularity's constructor. The API gateway only classifies UserError as a client error, so the REST API responds with 500 Internal Server Error for what is really invalid client input:

Error: Granularity "all" does not exist in dimension Sales.local_reporting_timestamp

This change throws UserError instead, so /v1/load returns 400 for unknown granularities, consistent with other invalid-query errors (unknown members, malformed filters, etc.).

Added a unit test in base-query.test.ts covering the unknown-granularity case; ran the full Custom granularities suite (35 tests) and the schema-compiler linter (0 errors).

…returns 400 instead of 500

An unknown granularity in a time dimension (e.g. granularity: "all")
is a client input error, but it was thrown as a plain Error, which the
API gateway reports as HTTP 500 Internal Server Error. Throw UserError
instead so the REST API returns 400 like other invalid-query errors.

Co-authored-by: Claude Code <noreply@anthropic.com>
Ai-assisted: true
@BrianSigafoos-SQ
BrianSigafoos-SQ requested a review from a team as a code owner July 17, 2026 20:17
@github-actions github-actions Bot added javascript Pull requests that update Javascript code pr:community Contribution from Cube.js community members. labels Jul 17, 2026
@ovr

ovr commented Jul 20, 2026

Copy link
Copy Markdown
Member

/bot-review

@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ovr's task in 49s —— View job


Code Review

Verdict: LGTM — small, correct, well-scoped fix. No blocking issues.

Full review

Summary

Swapping the plain Error for UserError in Granularity.ts:48 is the right fix: api-gateway distinguishes UserError (→ HTTP 400) from other exceptions (→ HTTP 500), and an unknown/invalid granularity value is unambiguously bad client input, consistent with how unknown members and malformed filters are already surfaced.

Correctness

  • Only the error type changes; message and control flow are identical, so no risk of masking other failures.
  • UserError is already imported at the top of Granularity.ts, and it's already used elsewhere in the same package, so no new dependency surface.
  • Constructor is the only throw site; nothing catches the previous plain Error specifically, so no callers need updating.

Tests

  • The added test in base-query.test.ts:866 asserts both the class (toThrow(UserError)) and the message — good, since the whole point of the PR is the class check.
  • granularity: 'all' is a realistic repro (matches the PR description about older clients).
  • Nit (non-blocking): the test is async but buildQuery is synchronous and compilers.compiler.compile() is the only awaited call — fine as is, matches the style of neighboring tests in the file.

Nice-to-haves (optional, not blocking)

  • Consider a companion assertion at the API-gateway layer that a query with an unknown granularity yields a 4xx — that would lock in the user-visible behavior this PR is really about. Not required; the unit test is sufficient for this package.
  • No changelog / docs update needed (bug fix, no API surface change).

Security / performance

  • N/A — pure error-type change.
• branch: `fix/unknown-granularity-user-error`

@ovr
ovr merged commit e8843dc into cube-js:master Jul 20, 2026
83 of 87 checks passed
@ovr

ovr commented Jul 20, 2026

Copy link
Copy Markdown
Member

Thank you @BrianSigafoos-SQ for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Pull requests that update Javascript code pr:community Contribution from Cube.js community members.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants