Add breadcrumbs to CDN Worker (#5552)

kamilkisiela · web-flow · commit fdbe81783f3d · 2024-08-29T10:16:57.000Z
diff --git a/packages/services/cdn-worker/src/artifact-handler.ts b/packages/services/cdn-worker/src/artifact-handler.ts
@@ -2,6 +2,7 @@ import * as itty from 'itty-router';
 import zod from 'zod';
 import { createAnalytics, type Analytics } from './analytics';
 import { type ArtifactStorageReader, type ArtifactsType } from './artifact-storage-reader';
+import { createBreadcrumb, type Breadcrumb } from './breadcrumbs';
 import { InvalidAuthKeyResponse, MissingAuthKeyResponse } from './errors';
 import { IsAppDeploymentActive } from './is-app-deployment-active';
 import type { KeyValidator } from './key-validation';
@@ -21,6 +22,7 @@ type ArtifactRequestHandler = {
   isKeyValid: KeyValidator;
   isAppDeploymentActive: IsAppDeploymentActive;
   analytics?: Analytics;
+  breadcrumb?: Breadcrumb;
   fallback?: (
     request: Request,
     params: { targetId: string; artifactType: string },
@@ -60,6 +62,7 @@ const authHeaderName = 'x-hive-cdn-key' as const;
 export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
   const router = itty.Router<itty.IRequest & Request>();
   const analytics = deps.analytics ?? createAnalytics();
+  const breadcrumb = deps.breadcrumb ?? createBreadcrumb();
 
   const authenticate = async (
     request: itty.IRequest & Request,
@@ -100,8 +103,13 @@ export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
 
     const params = parseResult.data;
 
+    breadcrumb(
+      `Artifact v1 handler (type=${params.artifactType}, targetId=${params.targetId}, contractName=${params.contractName})`,
+    );
+
     /** Legacy handling for old client SDK versions. */
     if (params.artifactType === 'schema') {
+      breadcrumb('Redirecting from /schema to /services');
       return createResponse(
         analytics,
         'Found.',
diff --git a/packages/services/cdn-worker/src/artifact-storage-reader.ts b/packages/services/cdn-worker/src/artifact-storage-reader.ts
@@ -1,6 +1,7 @@
 import zod from 'zod';
 import type { Analytics } from './analytics';
 import { AwsClient } from './aws';
+import type { Breadcrumb } from './breadcrumbs';
 
 export function buildArtifactStorageKey(
   targetId: string,
@@ -56,6 +57,8 @@ export function buildAppDeploymentIsEnabledKey(
  * Read an artifact/app deployment operation from S3.
  */
 export class ArtifactStorageReader {
+  private breadcrumb: Breadcrumb;
+
   constructor(
     private s3: {
       client: AwsClient;
@@ -68,9 +71,12 @@ export class ArtifactStorageReader {
       bucketName: string;
     } | null,
     private analytics: Analytics | null,
+    breadcrumb: Breadcrumb | null,
     /** Timeout in milliseconds for S3 read calls. */
     private timeout: number = 5_000,
-  ) {}
+  ) {
+    this.breadcrumb = breadcrumb ?? (() => {});
+  }
 
   /**
    * Perform a request to S3, with retries and optional mirror.
@@ -122,9 +128,11 @@ export class ArtifactStorageReader {
         },
       })
       .catch(err => {
+        this.breadcrumb('Failed to fetch from primary');
         if (!this.s3Mirror) {
           return Promise.reject(err);
         }
+        this.breadcrumb('Fetching from primary and mirror now');
         // Use two AbortSignals to avoid a situation
         // where Response.body is consumed,
         // but the request was aborted after being resolved.
@@ -201,11 +209,18 @@ export class ArtifactStorageReader {
       artifactType = 'sdl';
     }
 
+    this.breadcrumb(
+      `Reading artifact (targetId=${targetId}, artifactType=${artifactType}, contractName=${contractName})`,
+    );
+
     const key = buildArtifactStorageKey(targetId, artifactType, contractName);
 
+    this.breadcrumb(`Reading artifact from S3 key: ${key}`);
+
     const headers: HeadersInit = {};
 
     if (etagValue) {
+      this.breadcrumb('if-none-match detected');
       headers['if-none-match'] = etagValue;
     }
 
@@ -246,6 +261,8 @@ export class ArtifactStorageReader {
       } as const;
     }
 
+    this.breadcrumb(`Failed to read artifact`);
+
     const body = await response.text();
     throw new Error(`GET request failed with status ${response.status}: ${body}`);
   }
@@ -330,8 +347,10 @@ export class ArtifactStorageReader {
   }
 
   async readLegacyAccessKey(targetId: string) {
+    const key = ['cdn-legacy-keys', targetId].join('/');
+    this.breadcrumb(`Reading from S3 key: ${key}`);
     const response = await this.request({
-      key: ['cdn-legacy-keys', targetId].join('/'),
+      key,
       method: 'GET',
       onAttempt: args => {
         this.analytics?.track(
@@ -353,10 +372,11 @@ export class ArtifactStorageReader {
   }
 
   async readAccessKey(targetId: string, keyId: string) {
-    const s3KeyParts = ['cdn-keys', targetId, keyId];
+    const key = ['cdn-keys', targetId, keyId].join('/');
+    this.breadcrumb(`Reading from S3 key: ${key}`);
 
     const response = await this.request({
-      key: s3KeyParts.join('/'),
+      key,
       method: 'GET',
       onAttempt: args => {
         this.analytics?.track(
diff --git a/packages/services/cdn-worker/src/breadcrumbs.ts b/packages/services/cdn-worker/src/breadcrumbs.ts
@@ -0,0 +1,7 @@
+export type Breadcrumb = (message: string) => void;
+
+export function createBreadcrumb() {
+  return (message: string) => {
+    console.debug(message);
+  };
+}
diff --git a/packages/services/cdn-worker/src/dev.ts b/packages/services/cdn-worker/src/dev.ts
@@ -23,14 +23,18 @@ const s3 = {
 // eslint-disable-next-line no-process-env
 const PORT = process.env.PORT ? parseInt(process.env.PORT, 10) : 4010;
 
-const artifactStorageReader = new ArtifactStorageReader(s3, null, null);
+const artifactStorageReader = new ArtifactStorageReader(s3, null, null, null);
 
 const handleRequest = createRequestHandler({
   isKeyValid: createIsKeyValid({
     artifactStorageReader,
     getCache: null,
     waitUntil: null,
     analytics: null,
+    breadcrumb: null,
+    captureException(error) {
+      console.error(error);
+    },
   }),
   async getArtifactAction(targetId, contractName, artifactType, eTag) {
     return artifactStorageReader.readArtifact(targetId, contractName, artifactType, eTag);
@@ -52,6 +56,10 @@ const handleArtifactRequest = createArtifactRequestHandler({
     getCache: null,
     waitUntil: null,
     analytics: null,
+    breadcrumb: null,
+    captureException(error) {
+      console.error(error);
+    },
   }),
   isAppDeploymentActive: createIsAppDeploymentActive({
     artifactStorageReader,
diff --git a/packages/services/cdn-worker/src/handler.ts b/packages/services/cdn-worker/src/handler.ts
@@ -2,6 +2,7 @@ import { buildSchema, introspectionFromSchema } from 'graphql';
 import { Analytics, createAnalytics } from './analytics';
 import { GetArtifactActionFn } from './artifact-handler';
 import { ArtifactsType as ModernArtifactsType } from './artifact-storage-reader';
+import { Breadcrumb, createBreadcrumb } from './breadcrumbs';
 import {
   CDNArtifactNotFound,
   InvalidArtifactTypeResponse,
@@ -192,6 +193,7 @@ async function parseIncomingRequest(
   request: Request,
   keyValidator: KeyValidator,
   analytics: Analytics,
+  breadcrumb: Breadcrumb,
 ): Promise<
   | { error: Response }
   | {
@@ -239,6 +241,7 @@ async function parseIncomingRequest(
         legacyToModernArtifactTypeMap[artifactType],
     };
   } catch (e) {
+    breadcrumb(`Failed to validate key for ${targetId}, error: ${e}`);
     console.warn(`Failed to validate key for ${targetId}, error:`, e);
     return {
       error: new InvalidAuthKeyResponse(analytics, request),
@@ -255,15 +258,22 @@ interface RequestHandlerDependencies {
   isKeyValid: IsKeyValid;
   getArtifactAction: GetArtifactActionFn;
   analytics?: Analytics;
+  breadcrumb?: Breadcrumb;
   fetchText: (url: string) => Promise<string>;
 }
 
 export function createRequestHandler(deps: RequestHandlerDependencies) {
   const analytics = deps.analytics ?? createAnalytics();
+  const breadcrumb = deps.breadcrumb ?? createBreadcrumb();
   const artifactTypesHandlers = createArtifactTypesHandlers(analytics);
 
   return async (request: Request): Promise<Response> => {
-    const parsedRequest = await parseIncomingRequest(request, deps.isKeyValid, analytics);
+    const parsedRequest = await parseIncomingRequest(
+      request,
+      deps.isKeyValid,
+      analytics,
+      breadcrumb,
+    );
 
     if ('error' in parsedRequest) {
       return parsedRequest.error;
diff --git a/packages/services/cdn-worker/src/index.ts b/packages/services/cdn-worker/src/index.ts
@@ -76,20 +76,58 @@ const handler: ExportedHandler<Env> = {
       s3: env.S3_ANALYTICS,
     });
 
-    const artifactStorageReader = new ArtifactStorageReader(s3, s3Mirror, analytics);
+    const sentry = new Toucan({
+      dsn: env.SENTRY_DSN,
+      environment: env.SENTRY_ENVIRONMENT,
+      release: env.SENTRY_RELEASE,
+      dist: 'cdn-worker',
+      context: ctx,
+      request,
+      requestDataOptions: {
+        allowedHeaders: [
+          'user-agent',
+          'cf-ipcountry',
+          'accept-encoding',
+          'accept',
+          'x-real-ip',
+          'cf-connecting-ip',
+        ],
+        allowedSearchParams: /(.*)/,
+      },
+    });
+
+    const artifactStorageReader = new ArtifactStorageReader(
+      s3,
+      s3Mirror,
+      analytics,
+      (message: string) => sentry.addBreadcrumb({ message }),
+    );
 
     const isKeyValid = createIsKeyValid({
       waitUntil: p => ctx.waitUntil(p),
       getCache: () => caches.open('artifacts-auth'),
       artifactStorageReader,
       analytics,
+      breadcrumb(message: string) {
+        sentry.addBreadcrumb({
+          message,
+        });
+      },
+      captureException(error) {
+        sentry.captureException(error);
+      },
     });
 
     const handleRequest = createRequestHandler({
       async getArtifactAction(targetId, contractName, artifactType, eTag) {
         return artifactStorageReader.readArtifact(targetId, contractName, artifactType, eTag);
       },
       isKeyValid,
+      breadcrumb(message: string) {
+        sentry.addBreadcrumb({
+          message,
+        });
+      },
       analytics,
       async fetchText(url) {
         // Yeah, it's not globally defined, but it makes no sense to define it globally
@@ -134,6 +172,9 @@ const handler: ExportedHandler<Env> = {
     const handleArtifactRequest = createArtifactRequestHandler({
       isKeyValid,
       analytics,
+      breadcrumb(message: string) {
+        sentry.addBreadcrumb({ message });
+      },
       artifactStorageReader,
       isAppDeploymentActive: createIsAppDeploymentActive({
         artifactStorageReader,
@@ -164,31 +205,16 @@ const handler: ExportedHandler<Env> = {
       // Legacy CDN Handlers
       .get('*', handleRequest);
 
-    const sentry = new Toucan({
-      dsn: env.SENTRY_DSN,
-      environment: env.SENTRY_ENVIRONMENT,
-      release: env.SENTRY_RELEASE,
-      dist: 'cdn-worker',
-      context: ctx,
-      request,
-      requestDataOptions: {
-        allowedHeaders: [
-          'user-agent',
-          'cf-ipcountry',
-          'accept-encoding',
-          'accept',
-          'x-real-ip',
-          'cf-connecting-ip',
-        ],
-        allowedSearchParams: /(.*)/,
-      },
-    });
-
     try {
       return await router.handle(request, sentry.captureException.bind(sentry)).then(response => {
         if (response) {
           return response;
         }
+
+        sentry.addBreadcrumb({
+          message: 'No response from router',
+        });
+
         return createResponse(analytics, 'Not found', { status: 404 }, 'unknown', request);
       });
     } catch (error) {
diff --git a/packages/services/cdn-worker/src/key-validation.ts b/packages/services/cdn-worker/src/key-validation.ts
@@ -1,6 +1,7 @@
 import bcrypt from 'bcryptjs';
 import { Analytics } from './analytics';
 import { ArtifactStorageReader } from './artifact-storage-reader';
+import type { Breadcrumb } from './breadcrumbs';
 import { decodeCdnAccessTokenSafe, isCDNAccessToken } from './cdn-token';
 
 export type KeyValidator = (targetId: string, headerKey: string) => Promise<boolean>;
@@ -14,6 +15,8 @@ type CreateKeyValidatorDeps = {
   artifactStorageReader: ArtifactStorageReader;
   getCache: null | GetCache;
   analytics: null | Analytics;
+  breadcrumb: null | Breadcrumb;
+  captureException: (error: Error) => void;
 };
 
 export const createIsKeyValid =
@@ -223,7 +226,20 @@ async function handleCDNAccessToken(
     return withCache(false);
   }
 
-  const isValid = await bcrypt.compare(decodeResult.token.privateKey, await key.text());
+  const isValid = await bcrypt
+    .compare(
+      decodeResult.token.privateKey,
+      await key.text().catch(error => {
+        deps.breadcrumb?.('Failed to read body of key: ' + error.message);
+        deps.captureException(error);
+        return Promise.reject(error);
+      }),
+    )
+    .catch(error => {
+      deps.breadcrumb?.(`Failed to compare keys: ${error.message}`);
+      deps.captureException(error);
+      return Promise.reject(error);
+    });
 
   deps.analytics?.track(
     {
diff --git a/packages/services/cdn-worker/tests/cdn.spec.ts b/packages/services/cdn-worker/tests/cdn.spec.ts
diff --git a/packages/services/server/src/index.ts b/packages/services/server/src/index.ts
