fix(schema): always treat network errors as schema publish rejections (#6779)

n1ru4l · web-flow · commit edf528b310c8 · 2025-05-14T09:11:53.000+02:00
diff --git a/deployment/services/schema.ts b/deployment/services/schema.ts
@@ -42,7 +42,6 @@ export function deploySchema({
         SCHEMA_CACHE_POLL_INTERVAL_MS: '150',
         SCHEMA_CACHE_TTL_MS: '65000' /* 65s */,
         SCHEMA_CACHE_SUCCESS_TTL_MS: String(hourInMS * 2),
-        SCHEMA_COMPOSITION_TIMEOUT_MS: String(minuteInMS),
         OPENTELEMETRY_COLLECTOR_ENDPOINT:
           observability.enabled && observability.tracingEndpoint
             ? observability.tracingEndpoint
diff --git a/integration-tests/package.json b/integration-tests/package.json
@@ -21,7 +21,7 @@
     "@hive/schema": "workspace:*",
     "@hive/server": "workspace:*",
     "@hive/storage": "workspace:*",
-    "@theguild/federation-composition": "0.18.1",
+    "@theguild/federation-composition": "0.18.3",
     "@trpc/client": "10.45.2",
     "@trpc/server": "10.45.2",
     "@types/async-retry": "1.4.8",
diff --git a/integration-tests/tests/api/schema/external-composition.spec.ts b/integration-tests/tests/api/schema/external-composition.spec.ts
@@ -367,7 +367,7 @@ test.concurrent('a timeout error should be visible to the user', async ({ expect
         total: 1,
         nodes: [
           {
-            message: expect.stringMatching(/timeout/i),
+            message: expect.stringMatching(/The schema composition timed out. Please try again./i),
           },
         ],
       },
diff --git a/packages/libraries/cli/package.json b/packages/libraries/cli/package.json
@@ -59,7 +59,7 @@
     "@oclif/core": "^3.26.6",
     "@oclif/plugin-help": "6.0.22",
     "@oclif/plugin-update": "4.2.13",
-    "@theguild/federation-composition": "0.18.1",
+    "@theguild/federation-composition": "0.18.3",
     "colors": "1.4.0",
     "env-ci": "7.3.0",
     "graphql": "^16.8.1",
diff --git a/packages/services/api/package.json b/packages/services/api/package.json
@@ -37,7 +37,7 @@
     "@sentry/node": "7.120.2",
     "@sentry/types": "7.120.2",
     "@slack/web-api": "7.8.0",
-    "@theguild/federation-composition": "0.18.1",
+    "@theguild/federation-composition": "0.18.3",
     "@trpc/client": "10.45.2",
     "@trpc/server": "10.45.2",
     "@types/bcryptjs": "2.4.6",
diff --git a/packages/services/api/src/modules/schema/providers/models/composite.ts b/packages/services/api/src/modules/schema/providers/models/composite.ts
@@ -456,6 +456,21 @@ export class CompositeModel {
         })) ?? null,
     });
 
+    if (
+      compositionCheck.status === 'failed' &&
+      (compositionCheck.reason.includesNetworkError || compositionCheck.reason.includesException)
+    ) {
+      return {
+        conclusion: SchemaPublishConclusion.Reject,
+        reasons: [
+          {
+            code: PublishFailureReasonCode.CompositionFailure,
+            compositionErrors: compositionCheck.reason.errorsBySource.composition,
+          },
+        ],
+      };
+    }
+
     if (
       compositionCheck.status === 'failed' &&
       compositionCheck.reason.errorsBySource.graphql.length > 0 &&
diff --git a/packages/services/api/src/modules/schema/providers/orchestrators/federation.ts b/packages/services/api/src/modules/schema/providers/orchestrators/federation.ts
@@ -3,7 +3,13 @@ import { abortSignalAny } from '@graphql-hive/signal';
 import type { ContractsInputType, SchemaBuilderApi } from '@hive/schema';
 import { traceFn } from '@hive/service-common';
 import { createTRPCProxyClient, httpLink } from '@trpc/client';
-import { Orchestrator, Project, ProjectType, SchemaObject } from '../../../../shared/entities';
+import {
+  ComposeAndValidateResult,
+  Orchestrator,
+  Project,
+  ProjectType,
+  SchemaObject,
+} from '../../../../shared/entities';
 import { Logger } from '../../../shared/providers/logger';
 import type { SchemaServiceConfig } from './tokens';
 import { SCHEMA_SERVICE_CONFIG } from './tokens';
@@ -121,8 +127,29 @@ export class FederationOrchestrator implements Orchestrator {
           signal: abortSignalAny([this.incomingRequestAbortSignal, timeoutAbortSignal]),
         },
       );
-
       return result;
+    } catch (err) {
+      // In case of a timeout error we return something the user can process
+      if (timeoutAbortSignal.reason) {
+        return {
+          contracts: null,
+          metadataAttributes: null,
+          schemaMetadata: null,
+          sdl: null,
+          supergraph: null,
+          tags: null,
+          includesNetworkError: true,
+          includesException: false,
+          errors: [
+            {
+              message: 'The schema composition timed out. Please try again.',
+              source: 'composition',
+            },
+          ],
+        } satisfies ComposeAndValidateResult;
+      }
+
+      throw err;
     } finally {
       timeoutAbortSignal.removeEventListener('abort', onTimeout);
       this.incomingRequestAbortSignal.removeEventListener('abort', onIncomingRequestAbort);
diff --git a/packages/services/api/src/modules/schema/providers/orchestrators/single.ts b/packages/services/api/src/modules/schema/providers/orchestrators/single.ts
@@ -3,7 +3,12 @@ import { abortSignalAny } from '@graphql-hive/signal';
 import type { SchemaBuilderApi } from '@hive/schema';
 import { traceFn } from '@hive/service-common';
 import { createTRPCProxyClient, httpLink } from '@trpc/client';
-import { Orchestrator, ProjectType, SchemaObject } from '../../../../shared/entities';
+import {
+  ComposeAndValidateResult,
+  Orchestrator,
+  ProjectType,
+  SchemaObject,
+} from '../../../../shared/entities';
 import { Logger } from '../../../shared/providers/logger';
 import type { SchemaServiceConfig } from './tokens';
 import { SCHEMA_SERVICE_CONFIG } from './tokens';
@@ -90,6 +95,27 @@ export class SingleOrchestrator implements Orchestrator {
         },
       );
       return result;
+    } catch (err) {
+      // In case of a timeout error we return something the user can process
+      if (timeoutAbortSignal.reason) {
+        return {
+          contracts: null,
+          metadataAttributes: null,
+          schemaMetadata: null,
+          sdl: null,
+          supergraph: null,
+          tags: null,
+          includesNetworkError: true,
+          includesException: false,
+          errors: [
+            {
+              message: 'The schema composition timed out. Please try again.',
+              source: 'composition',
+            },
+          ],
+        } satisfies ComposeAndValidateResult;
+      }
+      throw err;
     } finally {
       timeoutAbortSignal.removeEventListener('abort', onTimeout);
       this.incomingRequestAbortSignal.removeEventListener('abort', onIncomingRequestAbort);
diff --git a/packages/services/api/src/modules/schema/providers/orchestrators/stitching.ts b/packages/services/api/src/modules/schema/providers/orchestrators/stitching.ts
@@ -3,7 +3,12 @@ import { abortSignalAny } from '@graphql-hive/signal';
 import type { SchemaBuilderApi } from '@hive/schema';
 import { traceFn } from '@hive/service-common';
 import { createTRPCProxyClient, httpLink } from '@trpc/client';
-import { Orchestrator, ProjectType, SchemaObject } from '../../../../shared/entities';
+import {
+  ComposeAndValidateResult,
+  Orchestrator,
+  ProjectType,
+  SchemaObject,
+} from '../../../../shared/entities';
 import { Logger } from '../../../shared/providers/logger';
 import type { SchemaServiceConfig } from './tokens';
 import { SCHEMA_SERVICE_CONFIG } from './tokens';
@@ -84,6 +89,27 @@ export class StitchingOrchestrator implements Orchestrator {
         },
       );
       return result;
+    } catch (err) {
+      // In case of a timeout error we return something the user can process
+      if (timeoutAbortSignal.reason) {
+        return {
+          contracts: null,
+          metadataAttributes: null,
+          schemaMetadata: null,
+          sdl: null,
+          supergraph: null,
+          tags: null,
+          includesNetworkError: true,
+          includesException: false,
+          errors: [
+            {
+              message: 'The schema composition timed out. Please try again.',
+              source: 'composition',
+            },
+          ],
+        } satisfies ComposeAndValidateResult;
+      }
+      throw err;
     } finally {
       timeoutAbortSignal.removeEventListener('abort', onTimeout);
       this.incomingRequestAbortSignal.removeEventListener('abort', onIncomingRequestAbort);
diff --git a/packages/services/api/src/modules/schema/providers/registry-checks.ts b/packages/services/api/src/modules/schema/providers/registry-checks.ts
@@ -283,6 +283,8 @@ export class RegistryChecks {
           // Federation 1 apparently has SDL and validation errors at the same time.
           fullSchemaSdl: result.sdl,
           contracts: result.contracts?.map(mapContract) ?? null,
+          includesNetworkError: result.includesNetworkError ?? false,
+          includesException: result.includesException ?? false,
         },
       } satisfies CheckResult;
     }
diff --git a/packages/services/api/src/shared/entities.ts b/packages/services/api/src/shared/entities.ts
@@ -394,6 +394,8 @@ export interface ComposeAndValidateResult {
     Array<{ name: string; content: string; source: string | null }>
   > | null;
   metadataAttributes: null | Record<string, string[]>;
+  includesNetworkError?: boolean;
+  includesException?: boolean;
 }
 
 export interface Orchestrator {
diff --git a/packages/services/schema/.env.template b/packages/services/schema/.env.template
@@ -4,5 +4,6 @@ REDIS_PASSWORD=""
 ENCRYPTION_SECRET="97e4094d2463e71a981913cca4e56788"
 SCHEMA_CACHE_TTL_MS=5000
 SCHEMA_CACHE_SUCCESS_TTL_MS=5000
-SCHEMA_COMPOSITION_TIMEOUT_MS=25000
-OPENTELEMETRY_COLLECTOR_ENDPOINT="<sync>"
+SCHEMA_COMPOSITION_TIMEOUT_MS=30000
+SCHEMA_EXTERNAL_COMPOSITION_TIMEOUT_MS=9000
+OPENTELEMETRY_COLLECTOR_ENDPOINT="<sync>"
diff --git a/packages/services/schema/package.json b/packages/services/schema/package.json
@@ -15,7 +15,7 @@
     "@graphql-tools/stitching-directives": "3.1.31",
     "@hive/service-common": "workspace:*",
     "@sentry/node": "7.120.2",
-    "@theguild/federation-composition": "0.18.1",
+    "@theguild/federation-composition": "0.18.3",
     "@trpc/server": "10.45.2",
     "@types/async-retry": "1.4.8",
     "@types/ioredis-mock": "8.2.5",
diff --git a/packages/services/schema/src/api.ts b/packages/services/schema/src/api.ts
@@ -28,7 +28,7 @@ const t = initTRPC.context<Context>().create();
 
 const procedure = t.procedure.use(handleTRPCError);
 
-const EXTERNAL_VALIDATION = z
+const ExternalValidationModel = z
   .object({
     endpoint: z.string().url().min(1),
     encryptedSecret: z.string().min(1),
@@ -72,7 +72,7 @@ export const schemaBuilderApiRouter = t.router({
               })
               .required(),
           ),
-          external: EXTERNAL_VALIDATION,
+          external: ExternalValidationModel,
           native: z.boolean().optional(),
           contracts: ContractsInputModel.nullable().optional(),
         }),
@@ -100,7 +100,6 @@ export const schemaBuilderApiRouter = t.router({
                 data: {
                   type: 'federation',
                   args,
-                  requestTimeoutMs: ctx.cache.timeoutMs,
                 },
                 abortSignal,
                 requestId: ctx.req.id,
@@ -159,24 +158,28 @@ export const schemaBuilderApiRouter = t.router({
 
         assertAllCasesExhausted(input);
       } catch (error) {
+        ctx.req.log.error('Composition timed out. (error=%o)', error);
+
         // Treat timeouts caused by external composition as "expected errors"
-        if (ctx.cache.isTimeoutError(error) && input.type === 'federation' && input.external) {
+        if (ctx.cache.isTimeoutError(error)) {
           return {
             errors: [
               {
-                message: error.message,
-                source: 'graphql',
+                message: 'The composition timed out. Please try again later.',
+                source: 'composition',
               },
             ],
             sdl: null,
             supergraph: null,
-            includesNetworkError: true,
+            includesNetworkError: false,
+            includesException: true,
             contracts: null,
             tags: null,
             schemaMetadata: null,
             metadataAttributes: null,
           } satisfies CompositionResponse;
         }
+
         throw error;
       }
 
@@ -205,4 +208,5 @@ export type CompositionResponse = {
   metadataAttributes: Record<string, string[]> | null;
   tags: Array<string> | null;
   includesNetworkError?: boolean;
+  includesException?: boolean;
 };
diff --git a/packages/services/schema/src/composition-worker.ts b/packages/services/schema/src/composition-worker.ts
@@ -40,7 +40,7 @@ export function createCompositionWorker(args: {
           const composeFederation = createComposeFederation({
             decrypt,
             logger: baseLogger.child({ reqId: message.data.args.requestId }) as FastifyBaseLogger,
-            requestTimeoutMs: message.data.requestTimeoutMs,
+            requestTimeoutMs: args.env.timings.schemaExternalCompositionTimeout,
           });
           const composed = await composeFederation(message.data.args);
 
@@ -128,7 +128,6 @@ export type CompositionEvent = {
     | {
         type: 'federation';
         args: ComposeFederationArgs;
-        requestTimeoutMs: number;
       }
     | {
         type: 'single';
diff --git a/packages/services/schema/src/environment.ts b/packages/services/schema/src/environment.ts
@@ -43,7 +43,8 @@ const RequestBrokerModel = zod.union([
 const TimingsModel = zod.object({
   SCHEMA_CACHE_TTL_MS: NumberFromString().default(30000),
   SCHEMA_CACHE_SUCCESS_TTL_MS: zod.optional(NumberFromString()),
-  SCHEMA_COMPOSITION_TIMEOUT_MS: NumberFromString(25000).default(25000),
+  SCHEMA_COMPOSITION_TIMEOUT_MS: NumberFromString(30_000).default(30_000),
+  SCHEMA_EXTERNAL_COMPOSITION_TIMEOUT_MS: NumberFromString(9_000).default(9_000),
   SCHEMA_CACHE_POLL_INTERVAL_MS: NumberFromString(100).default(150),
 });
 
@@ -175,7 +176,13 @@ export const env = {
       timings.SCHEMA_CACHE_SUCCESS_TTL_MS ||
       timings.SCHEMA_CACHE_TTL_MS /* Fallback to cacheTTL if not set */,
     cachePollInterval: timings.SCHEMA_CACHE_POLL_INTERVAL_MS,
+    /** timeout of the composition in worker */
     schemaCompositionTimeout: timings.SCHEMA_COMPOSITION_TIMEOUT_MS,
+    /**
+     * Timeout of calls to the external composition endpoint within worker
+     * NOTE: This value should always be lower than the schemaCompositionTimeout
+     */
+    schemaExternalCompositionTimeout: timings.SCHEMA_EXTERNAL_COMPOSITION_TIMEOUT_MS,
   },
   requestBroker:
     requestBroker.REQUEST_BROKER === '1'
diff --git a/packages/services/schema/src/lib/compose.ts b/packages/services/schema/src/lib/compose.ts
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml

Original file line number	Diff line number	Diff line change
`@@ -367,7 +367,7 @@ test.concurrent('a timeout error should be visible to the user', async ({ expect`
`367`	`367`	`total: 1,`
`368`	`368`	`nodes: [`
`369`	`369`	`{`
`370`		`- message: expect.stringMatching(/timeout/i),`
	`370`	`+ message: expect.stringMatching(/The schema composition timed out. Please try again./i),`
`371`	`371`	`},`
`372`	`372`	`],`
`373`	`373`	`},`
Original file line number	Diff line number	Diff line change
`@@ -283,6 +283,8 @@ export class RegistryChecks {`
`283`	`283`	`// Federation 1 apparently has SDL and validation errors at the same time.`
`284`	`284`	`fullSchemaSdl: result.sdl,`
`285`	`285`	`contracts: result.contracts?.map(mapContract) ?? null,`
	`286`	`+ includesNetworkError: result.includesNetworkError ?? false,`
	`287`	`+ includesException: result.includesException ?? false,`
`286`	`288`	`},`
`287`	`289`	`} satisfies CheckResult;`
`288`	`290`	`}`