fix(core): Add log level only to 4xx/5xx codes

Zen-cronic · Zen-cronic · commit bfa9cafdf154 · 2024-09-22T13:34:42.000-04:00
Modify `getBreadcrumbLogLevel` to account only for 4xx or 5xx status.
Tests updated to reflect the changes to `getBreadcrumbLogLevel` function.

Signed-off-by: Kaung Zin Hein &lt;kaungzinhein113@gmail.com&gt;
diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/get/test.ts b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/get/test.ts
@@ -33,6 +33,5 @@ sentryTest('captures Breadcrumb for basic GET request', async ({ getLocalTestUrl
       status_code: 200,
       url: 'http://sentry-test.io/foo',
     },
-    level: 'info',
   });
 });
diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/getWithRequestObj/test.ts b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/getWithRequestObj/test.ts
@@ -33,6 +33,5 @@ sentryTest('captures Breadcrumb for basic GET request that uses request object',
       status_code: 200,
       url: 'http://sentry-test.io/foo',
     },
-    level: 'info',
   });
 });
diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/post/test.ts b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/post/test.ts
@@ -33,6 +33,5 @@ sentryTest('captures Breadcrumb for POST request', async ({ getLocalTestUrl, pag
       status_code: 200,
       url: 'http://sentry-test.io/foo',
     },
-    level: 'info',
   });
 });
diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/subject.js b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/subject.js
@@ -0,0 +1,3 @@
+fetch('http://sentry-test.io/foo').then(() => {
+  Sentry.captureException('test error');
+});
diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/test.ts b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/test.ts
@@ -0,0 +1,71 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers';
+
+sentryTest('captures Breadcrumb with log level for 4xx response code', async ({ getLocalTestUrl, page }) => {
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.route('**/foo', async route => {
+    await route.fulfill({
+      status: 404,
+      contentType: 'text/plain',
+      body: 'Not Found!',
+    });
+  });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+
+  expect(eventData?.breadcrumbs?.length).toBe(1);
+  expect(eventData!.breadcrumbs![0]).toEqual({
+    timestamp: expect.any(Number),
+    category: 'fetch',
+    type: 'http',
+    data: {
+      method: 'GET',
+      status_code: 404,
+      url: 'http://sentry-test.io/foo',
+    },
+    level: 'warning',
+  });
+
+  await page.route('**/foo', async route => {
+    await route.fulfill({
+      status: 500,
+      contentType: 'text/plain',
+      body: 'Internal Server Error',
+    });
+  });
+});
+
+sentryTest('captures Breadcrumb with log level for 5xx response code', async ({ getLocalTestUrl, page }) => {
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.route('**/foo', async route => {
+    await route.fulfill({
+      status: 500,
+      contentType: 'text/plain',
+      body: 'Internal Server Error',
+    });
+  });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+
+  expect(eventData?.breadcrumbs?.length).toBe(1);
+  expect(eventData!.breadcrumbs![0]).toEqual({
+    timestamp: expect.any(Number),
+    category: 'fetch',
+    type: 'http',
+    data: {
+      method: 'GET',
+      status_code: 500,
+      url: 'http://sentry-test.io/foo',
+    },
+    level: 'error',
+  });
+});
diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/get/test.ts b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/get/test.ts
@@ -34,64 +34,5 @@ sentryTest('captures Breadcrumb for basic GET request', async ({ getLocalTestUrl
       status_code: 200,
       url: 'http://sentry-test.io/foo',
     },
-    level: 'info',
-  });
-});
-
-sentryTest('captures Breadcrumb for GET request with 4xx response code', async ({ getLocalTestUrl, page }) => {
-  const url = await getLocalTestUrl({ testDir: __dirname });
-
-  await page.route('**/foo', async route => {
-    await route.fulfill({
-      status: 404,
-      contentType: 'text/plain',
-      body: 'Not Found!',
-    });
-  });
-
-  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
-
-  expect(eventData.exception?.values).toHaveLength(1);
-
-  expect(eventData?.breadcrumbs?.length).toBe(1);
-  expect(eventData!.breadcrumbs![0]).toEqual({
-    timestamp: expect.any(Number),
-    category: 'xhr',
-    type: 'http',
-    data: {
-      method: 'GET',
-      status_code: 404,
-      url: 'http://sentry-test.io/foo',
-    },
-    level: 'warning',
-  });
-});
-
-sentryTest('captures Breadcrumb for GET request with 5xx response code', async ({ getLocalTestUrl, page }) => {
-  const url = await getLocalTestUrl({ testDir: __dirname });
-
-  await page.route('**/foo', async route => {
-    await route.fulfill({
-      status: 500,
-      contentType: 'text/plain',
-      body: 'Internal Server Error',
-    });
-  });
-
-  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
-
-  expect(eventData.exception?.values).toHaveLength(1);
-
-  expect(eventData?.breadcrumbs?.length).toBe(1);
-  expect(eventData!.breadcrumbs![0]).toEqual({
-    timestamp: expect.any(Number),
-    category: 'xhr',
-    type: 'http',
-    data: {
-      method: 'GET',
-      status_code: 500,
-      url: 'http://sentry-test.io/foo',
-    },
-    level: 'error',
   });
 });
diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/post/test.ts b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/post/test.ts
@@ -33,6 +33,5 @@ sentryTest('captures Breadcrumb for POST request', async ({ getLocalTestUrl, pag
       status_code: 200,
       url: 'http://sentry-test.io/foo',
     },
-    level: 'info',
   });
 });
diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/subject.js b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/subject.js
@@ -0,0 +1,10 @@
+const xhr = new XMLHttpRequest();
+
+xhr.open('GET', 'http://sentry-test.io/foo');
+xhr.send();
+
+xhr.addEventListener('readystatechange', function () {
+  if (xhr.readyState === 4) {
+    Sentry.captureException('test error');
+  }
+});
diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/test.ts b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/test.ts
@@ -0,0 +1,71 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers';
+
+sentryTest('captures Breadcrumb with log level for 4xx response code', async ({ getLocalTestUrl, page }) => {
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.route('**/foo', async route => {
+    await route.fulfill({
+      status: 404,
+      contentType: 'text/plain',
+      body: 'Not Found!',
+    });
+  });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+
+  expect(eventData?.breadcrumbs?.length).toBe(1);
+  expect(eventData!.breadcrumbs![0]).toEqual({
+    timestamp: expect.any(Number),
+    category: 'xhr',
+    type: 'http',
+    data: {
+      method: 'GET',
+      status_code: 404,
+      url: 'http://sentry-test.io/foo',
+    },
+    level: 'warning',
+  });
+
+  await page.route('**/foo', async route => {
+    await route.fulfill({
+      status: 500,
+      contentType: 'text/plain',
+      body: 'Internal Server Error',
+    });
+  });
+});
+
+sentryTest('captures Breadcrumb with log level for 5xx response code', async ({ getLocalTestUrl, page }) => {
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.route('**/foo', async route => {
+    await route.fulfill({
+      status: 500,
+      contentType: 'text/plain',
+      body: 'Internal Server Error',
+    });
+  });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+
+  expect(eventData?.breadcrumbs?.length).toBe(1);
+  expect(eventData!.breadcrumbs![0]).toEqual({
+    timestamp: expect.any(Number),
+    category: 'xhr',
+    type: 'http',
+    data: {
+      method: 'GET',
+      status_code: 500,
+      url: 'http://sentry-test.io/foo',
+    },
+    level: 'error',
+  });
+});
diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts
@@ -254,7 +254,7 @@ function _getXhrBreadcrumbHandler(client: Client): (handlerData: HandlerDataXhr)
         category: 'xhr',
         data,
         type: 'http',
-        level,
+        ...level,
       },
       hint,
     );
@@ -319,7 +319,7 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe
           category: 'fetch',
           data,
           type: 'http',
-          level,
+          ...level,
         },
         hint,
       );
diff --git a/packages/cloudflare/src/integrations/fetch.ts b/packages/cloudflare/src/integrations/fetch.ts
@@ -158,7 +158,7 @@ function createBreadcrumb(handlerData: HandlerDataFetch): void {
         category: 'fetch',
         data,
         type: 'http',
-        level,
+        ...level,
       },
       hint,
     );
diff --git a/packages/core/src/utils/breadcrumbsUtils.ts b/packages/core/src/utils/breadcrumbsUtils.ts
@@ -1,17 +1,17 @@
 import type { SeverityLevel } from '@sentry/types';
 
 /**
- * Determine a breadcrumb's log level based on the response status code
+ * Determine a breadcrumb's log level (only `warning` or `error`) based on the response status code
  * @param statusCode
  */
-export function getBreadcrumbLogLevel(statusCode: number | undefined): SeverityLevel {
+export function getBreadcrumbLogLevel(statusCode: number | undefined): { level?: SeverityLevel } {
   if (statusCode === undefined) {
-    return 'info';
+    return {};
   } else if (statusCode >= 400 && statusCode < 500) {
-    return 'warning';
+    return { level: 'warning' };
   } else if (statusCode >= 500) {
-    return 'error';
+    return { level: 'error' };
   } else {
-    return 'info';
+    return {};
   }
 }
diff --git a/packages/core/test/lib/utils/breadcrumbsUtils.test.ts b/packages/core/test/lib/utils/breadcrumbsUtils.test.ts
@@ -1,15 +1,14 @@
-import type { SeverityLevel } from '@sentry/types';
 import { getBreadcrumbLogLevel } from '../../../src/utils/breadcrumbsUtils';
 
 describe('getBreadcrumbLogLevel()', () => {
   it.each([
-    ['warning', '4xx', 403],
-    ['error', '5xx', 500],
-    ['info', '3xx', 307],
-    ['info', '2xx', 200],
-    ['info', '1xx', 103],
-    ['info', 'undefined', undefined],
-  ] as [SeverityLevel, string, number | undefined][])('should return `%s` for %s', (output, _codeRange, input) => {
-    expect(getBreadcrumbLogLevel(input)).toBe(output);
+    [{ level: 'warning' }, '4xx', 403],
+    [{ level: 'error' }, '5xx', 500],
+    [{}, '3xx', 307],
+    [{}, '2xx', 200],
+    [{}, '1xx', 103],
+    [{}, 'undefined', undefined],
+  ])('should return `%s` for %s', (output, _codeRange, input) => {
+    expect(getBreadcrumbLogLevel(input)).toEqual(output);
   });
 });
diff --git a/packages/deno/src/integrations/breadcrumbs.ts b/packages/deno/src/integrations/breadcrumbs.ts
@@ -185,7 +185,7 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe
           category: 'fetch',
           data,
           type: 'http',
-          level,
+          ...level,
         },
         hint,
       );
diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts
@@ -241,7 +241,7 @@ function _addRequestBreadcrumb(
         ...data,
       },
       type: 'http',
-      level,
+      ...level,
     },
     {
       event: 'response',
diff --git a/packages/node/src/integrations/node-fetch.ts b/packages/node/src/integrations/node-fetch.ts
@@ -72,7 +72,7 @@ function addRequestBreadcrumb(request: UndiciRequest, response: UndiciResponse):
         ...data,
       },
       type: 'http',
-      level,
+      ...level,
     },
     {
       event: 'response',
diff --git a/packages/vercel-edge/src/integrations/wintercg-fetch.ts b/packages/vercel-edge/src/integrations/wintercg-fetch.ts
@@ -164,7 +164,7 @@ function createBreadcrumb(handlerData: HandlerDataFetch): void {
         category: 'fetch',
         data,
         type: 'http',
-        level,
+        ...level,
       },
       hint,
     );

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+fetch('http://sentry-test.io/foo').then(() => {`
	`2`	`+ Sentry.captureException('test error');`
	`3`	`+});`