chore(deleter): improve observability and logging flow (#1037)

Remove some unnecessarily specific test assertions about logging statements. Add http breadcrums and server logs for failed requests.
Pocket · Jan 29, 2025 · 96a0f65 · 96a0f65
1 parent 3bda8fe
commit 96a0f65
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 21 deletions.
diff --git a/lambdas/account-data-deleter-events/src/handlers/accountDelete/index.spec.ts b/lambdas/account-data-deleter-events/src/handlers/accountDelete/index.spec.ts
@@ -53,12 +53,10 @@ describe('accountDelete handler', () => {
     nock(config.endpoint)
       .post(config.queueDeletePath)
       .reply(400, { errors: ['this is an error'] });
-    expect.assertions(3); // since it's in a try/catch, make sure we assert
+    expect.assertions(1); // since it's in a try/catch, make sure we assert
     try {
       await accountDeleteHandler(record as SQSRecord);
     } catch (e) {
-      expect(e.message).toContain('queueDelete - 400');
-      expect(e.message).toContain('this is an error');
       expect(e.errors.length).toEqual(1);
     }
   });
@@ -68,12 +66,10 @@ describe('accountDelete handler', () => {
       .post(config.stripeDeletePath)
       .reply(400, { errors: ['bad call'] });
     nock(config.endpoint).post(config.queueDeletePath).reply(200);
-    expect.assertions(3);
+    expect.assertions(1);
     try {
       await accountDeleteHandler(record as SQSRecord);
     } catch (e) {
-      expect(e.message).toContain('stripeDelete - 400');
-      expect(e.message).toContain('bad call');
       expect(e.errors.length).toEqual(1);
     }
   });
@@ -86,14 +82,10 @@ describe('accountDelete handler', () => {
       .post(config.queueDeletePath)
       .reply(400, { errors: ['this is an error'] });
 
-    expect.assertions(5);
+    expect.assertions(1);
     try {
       await accountDeleteHandler(record as SQSRecord);
     } catch (e) {
-      expect(e.message).toContain('stripeDelete - 400');
-      expect(e.message).toContain('bad call');
-      expect(e.message).toContain('queueDelete - 400');
-      expect(e.message).toContain('this is an error');
       expect(e.errors.length).toEqual(2);
     }
   });

diff --git a/lambdas/account-data-deleter-events/src/handlers/accountDelete/index.ts b/lambdas/account-data-deleter-events/src/handlers/accountDelete/index.ts
@@ -1,3 +1,4 @@
+import * as Sentry from '@sentry/aws-serverless';
 import { AccountDeleteEvent } from '../../schemas/accountDeleteEvent.ts';
 import {
   callQueueDeleteEndpoint,
@@ -6,6 +7,8 @@ import {
 } from './postRequest.ts';
 import { SQSRecord } from 'aws-lambda';
 import { AggregateError } from '../../errors/AggregateError.ts';
+import { config } from '../../config.ts';
+import { serverLogger } from '@pocket-tools/ts-logger';
 
 type AccountDeleteBody = {
   userId: string;
@@ -64,17 +67,28 @@ export async function accountDeleteHandler(record: SQSRecord): Promise<void> {
   const fxaRes = await callFxARevokeEndpoint(postBody);
   const errors: Error[] = [];
   for await (const { endpoint, res } of [
-    { endpoint: 'queueDelete', res: queueRes },
-    { endpoint: 'stripeDelete', res: stripeRes },
-    { endpoint: 'fxaDelete', res: fxaRes },
+    { endpoint: config.queueDeletePath, res: queueRes },
+    { endpoint: config.stripeDeletePath, res: stripeRes },
+    { endpoint: config.fxaRevokePath, res: fxaRes },
   ]) {
+    Sentry.addBreadcrumb({
+      type: 'http',
+      data: {
+        status_code: res.status,
+        reason: res.statusText,
+        url: endpoint,
+        payload: postBody,
+      },
+    });
     if (!res.ok) {
-      const data = await res.json();
-      errors.push(
-        new Error(
-          `${endpoint} - ${res.status}\n${JSON.stringify(data.errors)}`,
-        ),
-      );
+      serverLogger.error({
+        message: 'HTTP request failed',
+        endpoint,
+        payload: postBody,
+        status: res.status,
+        reason: res.statusText,
+      });
+      errors.push(new Error(`${endpoint} - ${res.status}: ${res.statusText}}`));
     }
   }
   if (errors.length) {

diff --git a/lambdas/account-data-deleter-events/src/handlers/accountDelete/postRequest.ts b/lambdas/account-data-deleter-events/src/handlers/accountDelete/postRequest.ts
@@ -15,7 +15,7 @@ async function postRequest(
   body: any,
   path: string,
   endpoint?: string,
-): Promise<any> {
+): Promise<Response> {
   const fetchPath = (endpoint ?? config.endpoint) + path;
   return newFetch(fetchPath, {
     retryOn: [500, 502, 503],