fix(search-api-graphql): tell the caller which argument they got wrong

docs/reference/search-api-graphql.md

@@ -310,6 +310,39 @@ option (default 100); a request outside `1 ≤ perPage ≤ maxPerPage` or with
 are fetched (and `page` pins to 1), so a filter UI can refresh its facet
 counts without paying for a page of results.
 
+Both bounds are stated in the arguments’ SDL descriptions, so the playground’s
+own documentation answers “how large may a page be?” before a request has to
+fail to say it.
+
+## Errors the caller can fix
+
+An invalid argument comes back as an ordinary GraphQL error carrying the
+sentence that says what was wrong, plus the conventional code:
+
+```json
+{
+  "errors": [
+    {
+      "message": "perPage must be between 0 and 100; got 150.",
+      "path": ["datasets"],
+      "extensions": { "code": "BAD_USER_INPUT" }
+    }
+  ]
+}
+```
+
+The code is what lets a client tell “fix your query” from “retry later” without
+matching on prose. Everything reported that way is **caller-fixable**: the
+paging bounds above, and a value rejected by the [`IRI`
+scalar](#finding-which-fields-accept-an-iri).
+
+Anything else is masked to `"Unexpected error."` – graphql-yoga’s default, and
+the right one for a fault the consumer can do nothing about (an unreachable
+engine, a bug here). Those are logged server-side with their stack; the caller
+gets no detail, because there is no detail they could act on. So a
+presentation-layer developer building against a hosted endpoint never has to
+read the API container’s log to learn that they sent something invalid.
+
 ## Guarding the contract
 
 Why the API, the index and a future REST surface cannot drift apart is the

packages/search-api-graphql/src/build-schema.ts

@@ -153,15 +153,16 @@ const iriScalar = new GraphQLScalarType<string, string>({
 
 function assertIri(value: string): string {
   if (!isAbsoluteIri(value)) {
-    throw new GraphQLError(
+    throw userError(
       `IRI cannot represent “${value}”: an IRI needs a scheme (for example “https:”, “urn:” or “doi:”) and no whitespace. A value like this is usually a label or a token, which selects nothing on a field that keys on identity.`,
     );
   }
   return value;
 }
 
 /** Outbound the fault is the index, not the caller, so the message points at
- *  the fix rather than at the query. */
+ *  the fix rather than at the query – and it carries no `BAD_USER_INPUT`,
+ *  since there is nothing the query could have done differently. */
 function assertIriOut(value: string): string {
   if (!isAbsoluteIri(value)) {
     throw new GraphQLError(
@@ -171,6 +172,27 @@ function assertIriOut(value: string): string {
   return value;
 }
 
+/**
+ * An error the CALLER can fix, marked as such: a `GraphQLError` carrying
+ * `extensions.code = 'BAD_USER_INPUT'`.
+ *
+ * Both halves matter, and both are about the consumer rather than about us. A
+ * plain `Error` thrown from a resolver is a server fault as far as the
+ * transport is concerned, so graphql-yoga masks it to `“Unexpected error.”`;
+ * the message then survives only in the API container’s log, where a
+ * presentation-layer developer building against a hosted endpoint cannot read
+ * it. Throwing a `GraphQLError` keeps the sentence that says what was wrong.
+ * The `code` says whose fault it is, so a client can distinguish “fix your
+ * query” from “retry later” without matching on prose – the convention every
+ * major GraphQL server shares. `“Unexpected error.”` is left for what the name
+ * says: faults we did not anticipate.
+ */
+function userError(message: string): GraphQLError {
+  return new GraphQLError(message, {
+    extensions: { code: 'BAD_USER_INPUT' },
+  });
+}
+
 /** SCREAMING_SNAKE_CASE for an enum value name, e.g. `datePosted` → `DATE_POSTED`. */
 function screamingSnake(name: string): string {
   return name.replace(/([a-z0-9])([A-Z])/g, '$1_$2').toUpperCase();
@@ -737,11 +759,32 @@ export function buildGraphQLSchema(
     return {
       type: new GraphQLNonNull(resultType),
       args: {
-        query: { type: GraphQLString },
-        where: { type: whereInput },
-        orderBy: { type: orderByInput },
-        page: { type: GraphQLInt, defaultValue: 1 },
-        perPage: { type: GraphQLInt, defaultValue: 20 },
+        query: {
+          type: GraphQLString,
+          description: 'Free-text query. Omit it to browse by filter alone.',
+        },
+        where: {
+          type: whereInput,
+          description: 'Conditions every result must satisfy.',
+        },
+        orderBy: {
+          type: orderByInput,
+          description:
+            'Sort order. Defaults to relevance for a free-text query.',
+        },
+        page: {
+          type: GraphQLInt,
+          defaultValue: 1,
+          // The bound in the SDL, so the playground’s own documentation
+          // answers “how large may a page be?” before a request has to fail
+          // to say it.
+          description: '1-based page number; at least 1.',
+        },
+        perPage: {
+          type: GraphQLInt,
+          defaultValue: 20,
+          description: `Results per page, between 0 and ${maxPerPage}. Use 0 for a facet-only query: no results are fetched, and the facet counts still come back.`,
+        },
       },
       resolve: async (_source, args, context: SearchContext) => {
         const built = argsToQuery(
@@ -871,11 +914,11 @@ function argsToQuery(
   const perPage = args.perPage ?? 20;
   const page = args.page ?? 1;
   if (page < 1) {
-    throw new Error(`page must be at least 1; got ${page}.`);
+    throw userError(`page must be at least 1; got ${page}.`);
   }
   // perPage: 0 is a legitimate facet-only query (no hits, page pins to 1).
   if (perPage < 0 || perPage > maxPerPage) {
-    throw new Error(
+    throw userError(
       `perPage must be between 0 and ${maxPerPage}; got ${perPage}.`,
     );
   }

packages/search-api-graphql/test/__snapshots__/generator-stability.test.ts.snap

@@ -2,7 +2,24 @@
 
 exports[`GraphQL generator stability > emits a stable SDL for a representative schema 1`] = `
 "type Query {
-  things(query: String, where: ThingWhere, orderBy: ThingOrderBy, page: Int = 1, perPage: Int = 20): ThingSearchResult!
+  things(
+    """Free-text query. Omit it to browse by filter alone."""
+    query: String
+
+    """Conditions every result must satisfy."""
+    where: ThingWhere
+
+    """Sort order. Defaults to relevance for a free-text query."""
+    orderBy: ThingOrderBy
+
+    """1-based page number; at least 1."""
+    page: Int = 1
+
+    """
+    Results per page, between 0 and 100. Use 0 for a facet-only query: no results are fetched, and the facet counts still come back.
+    """
+    perPage: Int = 20
+  ): ThingSearchResult!
 }
 
 type ThingSearchResult {

packages/search-api-graphql/test/build-schema.test.ts

@@ -225,6 +225,11 @@ describe('buildGraphQLSchema', () => {
     expect(badPerPage.errors?.[0]?.message).toMatch(
       /perPage must be between 0 and 100/,
     );
+    // Marked caller-fixable, so a transport keeps the message rather than
+    // masking it as a server fault (see handler.test.ts).
+    for (const error of [badPage.errors?.[0], badPerPage.errors?.[0]]) {
+      expect(error?.extensions.code).toBe('BAD_USER_INPUT');
+    }
   });
 
   it('orders the output list best-first for the requested language', async () => {

packages/search-api-graphql/test/handler.test.ts

@@ -103,6 +103,49 @@ describe('createSearchGraphQLHandler', () => {
     expect(received().text).toBe('kaart');
   });
 
+  // The regression the unit tests cannot catch: argument validation runs inside
+  // a resolver, and the transport masks a resolver throw as “Unexpected error.”
+  // unless it is a GraphQLError. Asserted through the handler, since masking is
+  // exactly what the direct-execution tests bypass.
+  it('reports an invalid argument to the caller instead of masking it', async () => {
+    const { engine } = fakeEngine();
+    const handler = createSearchGraphQLHandler({
+      searchSchema: searchSchema(schema),
+      engine,
+    });
+
+    const response = await post(
+      handler,
+      '{ datasets(perPage: 150) { pagination { total } } }',
+    );
+    const { errors } = await response.json();
+
+    expect(errors[0].message).toBe(
+      'perPage must be between 0 and 100; got 150.',
+    );
+    expect(errors[0].extensions.code).toBe('BAD_USER_INPUT');
+  });
+
+  it('masks an engine failure, which the caller cannot fix', async () => {
+    const { engine } = fakeEngine();
+    const handler = createSearchGraphQLHandler({
+      searchSchema: searchSchema(schema),
+      engine: {
+        ...engine,
+        search: () => Promise.reject(new Error('connect ECONNREFUSED')),
+      },
+    });
+
+    const response = await post(
+      handler,
+      '{ datasets { pagination { total } } }',
+    );
+    const { errors } = await response.json();
+
+    expect(errors[0].message).toBe('Unexpected error.');
+    expect(errors[0].message).not.toContain('ECONNREFUSED');
+  });
+
   it('orders output languages by the Accept-Language header', async () => {
     const { engine, received } = fakeEngine();
     const handler = createSearchGraphQLHandler({

packages/search-api-graphql/test/print-sdl.test.ts

@@ -97,9 +97,10 @@ describe('printSchemaModuleSdl', () => {
       modulePath: fixture('no-options.mjs'),
     });
 
-    // Prettier breaks the multi-argument root field over several lines, so a
-    // surface move stays one added line in the committed diff.
-    expect(unformatted).toContain('  datasets(query: String');
+    // Prettier expands every one-line description into a block, so a wording
+    // change stays one changed line in the committed diff.
+    expect(unformatted).toContain('"""1-based page number; at least 1."""');
+    expect(formatted).toContain('    1-based page number; at least 1.\n');
     expect(formatted).not.toBe(unformatted);
   });