create test that actually failed previously

getsentry · Dec 21, 2023 · 722683d · 722683d
1 parent 7e6d2ae
commit 722683d
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 12 deletions.
diff --git a/.../browser-integration-tests/suites/public-api/startSpan/error-async-throw-ii/template.html b/.../browser-integration-tests/suites/public-api/startSpan/error-async-throw-ii/template.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+    <title></title>
+  </head>
+  <body>
+    <button id="button1" type="button">Button 1</button>
+
+    <script type="module">
+      async function run() {
+        Sentry.startSpan({ name: 'parent_span', op: 'test' }, async () => {
+          console.log('throwing error')
+          throw new Error('Async Thrown Error');
+        });
+      }
+
+      const button = document.getElementById('button1');
+      button.addEventListener('click', () => {
+        void run();
+        console.log('click');
+      });
+    </script>
+  </body>
+</html>
diff --git a/packages/browser-integration-tests/suites/public-api/startSpan/error-async-throw-ii/test.ts b/packages/browser-integration-tests/suites/public-api/startSpan/error-async-throw-ii/test.ts
@@ -0,0 +1,27 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getMultipleSentryEnvelopeRequests, shouldSkipTracingTest } from '../../../../utils/helpers';
+
+sentryTest(
+  "should capture a thrown error within an async startSpan callback that's not awaited properly",
+  async ({ getLocalTestPath, page }) => {
+    if (shouldSkipTracingTest()) {
+      sentryTest.skip();
+    }
+    const envelopePromise = getMultipleSentryEnvelopeRequests<Event>(page, 2);
+
+    const url = await getLocalTestPath({ testDir: __dirname });
+    await page.goto(url);
+
+    const clickPromise = page.getByText('Button 1').click();
+
+    // awaiting both events simultaneously to avoid race conditions
+    const [, events] = await Promise.all([clickPromise, envelopePromise]);
+    const [txn, err] = events[0]?.type === 'transaction' ? [events[0], events[1]] : [events[1], events[0]];
+
+    expect(txn).toMatchObject({ transaction: 'parent_span' });
+    expect(err?.exception?.values?.[0]?.value).toBe('Async Thrown Error');
+  },
+);
diff --git a/...ges/browser-integration-tests/suites/public-api/startSpan/error-async-throw/template.html b/...ges/browser-integration-tests/suites/public-api/startSpan/error-async-throw/template.html
@@ -7,16 +7,19 @@
   <body>
     <button id="button1" type="button">Button 1</button>
 
-    <script>
+    <script type="module">
       async function run() {
+        await Promise.resolve();
         await Sentry.startSpan({ name: 'parent_span', op: 'test' }, async () => {
+          console.log('throwing error')
           throw new Error('Async Thrown Error');
         });
       }
 
       const button = document.getElementById('button1');
-      button.addEventListener('click', async () => {
-        await run();
+      button.addEventListener('click', () => {
+        void run();
+        console.log('click');
       });
     </script>
   </body>

diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts
@@ -89,9 +89,9 @@ export function trace<T>(
  * and the `span` returned from the callback will be undefined.
  */
 export function startSpan<T>(context: TransactionContext, callback: (span: Span | undefined) => T): T {
-  console.log('START SPAN 2');
   const ctx = normalizeContext(context);
 
+  // @ts-expect-error - wtf
   return withScope(scope => {
     const hub = getCurrentHub();
     const parentSpan = scope.getSpan();
@@ -113,20 +113,21 @@ export function startSpan<T>(context: TransactionContext, callback: (span: Span
     }
 
     if (isThenable(maybePromiseResult)) {
-      Promise.resolve(maybePromiseResult).then(
-        () => {
+      return maybePromiseResult.then(
+        res => {
           finishAndSetSpan();
+          return res;
         },
-        () => {
+        e => {
           activeSpan && activeSpan.setStatus('internal_error');
           finishAndSetSpan();
+          throw e;
         },
       );
     } else {
       finishAndSetSpan();
+      return maybePromiseResult;
     }
-
-    return maybePromiseResult;
   });
 }
 
@@ -172,9 +173,13 @@ export function startSpanManual<T>(
     }
 
     if (isThenable(maybePromiseResult)) {
-      Promise.resolve(maybePromiseResult).then(undefined, () => {
-        activeSpan && activeSpan.setStatus('internal_error');
-      });
+      maybePromiseResult.then(
+        res => res,
+        e => {
+          activeSpan && activeSpan.setStatus('internal_error');
+          throw e;
+        },
+      );
     }
 
     return maybePromiseResult;