fix(sveltekit): Flush in server wrappers before exiting (#9153)

Lms24 · web-flow · commit 945d873ec980 · 2023-10-04T15:21:31.000+02:00
When deploying to Vercel (non-edge), server code is converted into
Lambda functions. These functions have a tendency to freeze/shut down
before the SDK is able to flush out events, causing events to be
dropped. We already handle this in NextJS by manually calling `flush` 
if we detect we're in a serverless/Lambda environment.
This patch applies the (almost) same logic to SvelteKit's server side
instrumentation.
diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts
@@ -1,12 +1,12 @@
 /* eslint-disable @sentry-internal/sdk/no-optional-chaining */
 import type { Span } from '@sentry/core';
-import { getActiveTransaction, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core';
+import { getActiveTransaction, getCurrentHub, runWithAsyncContext, startSpan } from '@sentry/core';
 import { captureException } from '@sentry/node';
 import { addExceptionMechanism, dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils';
 import type { Handle, ResolveOptions } from '@sveltejs/kit';
 
 import { isHttpError, isRedirect } from '../common/utils';
-import { getTracePropagationData } from './utils';
+import { flushIfServerless, getTracePropagationData } from './utils';
 
 export type SentryHandleOptions = {
   /**
@@ -118,33 +118,43 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
   return sentryRequestHandler;
 }
 
-function instrumentHandle({ event, resolve }: Parameters<Handle>[0], options: SentryHandleOptions): ReturnType<Handle> {
+async function instrumentHandle(
+  { event, resolve }: Parameters<Handle>[0],
+  options: SentryHandleOptions,
+): Promise<Response> {
   if (!event.route?.id && !options.handleUnknownRoutes) {
     return resolve(event);
   }
 
   const { dynamicSamplingContext, traceparentData, propagationContext } = getTracePropagationData(event);
   getCurrentHub().getScope().setPropagationContext(propagationContext);
 
-  return trace(
-    {
-      op: 'http.server',
-      origin: 'auto.http.sveltekit',
-      name: `${event.request.method} ${event.route?.id || event.url.pathname}`,
-      status: 'ok',
-      ...traceparentData,
-      metadata: {
-        source: event.route?.id ? 'route' : 'url',
-        dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
+  try {
+    const resolveResult = await startSpan(
+      {
+        op: 'http.server',
+        origin: 'auto.http.sveltekit',
+        name: `${event.request.method} ${event.route?.id || event.url.pathname}`,
+        status: 'ok',
+        ...traceparentData,
+        metadata: {
+          source: event.route?.id ? 'route' : 'url',
+          dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
+        },
+      },
+      async (span?: Span) => {
+        const res = await resolve(event, { transformPageChunk });
+        if (span) {
+          span.setHttpStatus(res.status);
+        }
+        return res;
       },
-    },
-    async (span?: Span) => {
-      const res = await resolve(event, { transformPageChunk });
-      if (span) {
-        span.setHttpStatus(res.status);
-      }
-      return res;
-    },
-    sendErrorToSentry,
-  );
+    );
+    return resolveResult;
+  } catch (e: unknown) {
+    sendErrorToSentry(e);
+    throw e;
+  } finally {
+    await flushIfServerless();
+  }
 }
diff --git a/packages/sveltekit/src/server/handleError.ts b/packages/sveltekit/src/server/handleError.ts
@@ -6,6 +6,8 @@ import { addExceptionMechanism } from '@sentry/utils';
 // eslint-disable-next-line import/no-unresolved
 import type { HandleServerError, RequestEvent } from '@sveltejs/kit';
 
+import { flushIfServerless } from './utils';
+
 // The SvelteKit default error handler just logs the error's stack trace to the console
 // see: https://github.com/sveltejs/kit/blob/369e7d6851f543a40c947e033bfc4a9506fdc0a8/packages/kit/src/runtime/server/index.js#L43
 function defaultErrorHandler({ error }: Parameters<HandleServerError>[0]): ReturnType<HandleServerError> {
@@ -20,7 +22,7 @@ function defaultErrorHandler({ error }: Parameters<HandleServerError>[0]): Retur
  * @param handleError The original SvelteKit error handler.
  */
 export function handleErrorWithSentry(handleError: HandleServerError = defaultErrorHandler): HandleServerError {
-  return (input: { error: unknown; event: RequestEvent }): ReturnType<HandleServerError> => {
+  return async (input: { error: unknown; event: RequestEvent }): Promise<void | App.Error> => {
     if (isNotFoundError(input)) {
       return handleError(input);
     }
@@ -36,6 +38,8 @@ export function handleErrorWithSentry(handleError: HandleServerError = defaultEr
       return scope;
     });
 
+    await flushIfServerless();
+
     return handleError(input);
   };
 }
diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts
@@ -1,13 +1,13 @@
 /* eslint-disable @sentry-internal/sdk/no-optional-chaining */
-import { getCurrentHub, trace } from '@sentry/core';
+import { getCurrentHub, startSpan } from '@sentry/core';
 import { captureException } from '@sentry/node';
 import type { TransactionContext } from '@sentry/types';
 import { addExceptionMechanism, addNonEnumerableProperty, objectify } from '@sentry/utils';
 import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit';
 
 import type { SentryWrappedFlag } from '../common/utils';
 import { isHttpError, isRedirect } from '../common/utils';
-import { getTracePropagationData } from './utils';
+import { flushIfServerless, getTracePropagationData } from './utils';
 
 type PatchedLoadEvent = LoadEvent & SentryWrappedFlag;
 type PatchedServerLoadEvent = ServerLoadEvent & SentryWrappedFlag;
@@ -57,7 +57,7 @@ function sendErrorToSentry(e: unknown): unknown {
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T): T {
   return new Proxy(origLoad, {
-    apply: (wrappingTarget, thisArg, args: Parameters<T>) => {
+    apply: async (wrappingTarget, thisArg, args: Parameters<T>) => {
       // Type casting here because `T` cannot extend `Load` (see comment above function signature)
       // Also, this event possibly already has a sentry wrapped flag attached
       const event = args[0] as PatchedLoadEvent;
@@ -80,7 +80,15 @@ export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T)
         },
       };
 
-      return trace(traceLoadContext, () => wrappingTarget.apply(thisArg, args), sendErrorToSentry);
+      try {
+        // We need to await before returning, otherwise we won't catch any errors thrown by the load function
+        return await startSpan(traceLoadContext, () => wrappingTarget.apply(thisArg, args));
+      } catch (e) {
+        sendErrorToSentry(e);
+        throw e;
+      } finally {
+        await flushIfServerless();
+      }
     },
   });
 }
@@ -109,7 +117,7 @@ export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T)
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export function wrapServerLoadWithSentry<T extends (...args: any) => any>(origServerLoad: T): T {
   return new Proxy(origServerLoad, {
-    apply: (wrappingTarget, thisArg, args: Parameters<T>) => {
+    apply: async (wrappingTarget, thisArg, args: Parameters<T>) => {
       // Type casting here because `T` cannot extend `ServerLoad` (see comment above function signature)
       // Also, this event possibly already has a sentry wrapped flag attached
       const event = args[0] as PatchedServerLoadEvent;
@@ -144,7 +152,15 @@ export function wrapServerLoadWithSentry<T extends (...args: any) => any>(origSe
         ...traceparentData,
       };
 
-      return trace(traceLoadContext, () => wrappingTarget.apply(thisArg, args), sendErrorToSentry);
+      try {
+        // We need to await before returning, otherwise we won't catch any errors thrown by the load function
+        return await startSpan(traceLoadContext, () => wrappingTarget.apply(thisArg, args));
+      } catch (e: unknown) {
+        sendErrorToSentry(e);
+        throw e;
+      } finally {
+        await flushIfServerless();
+      }
     },
   });
 }
diff --git a/packages/sveltekit/src/server/utils.ts b/packages/sveltekit/src/server/utils.ts
@@ -1,5 +1,6 @@
+import { flush } from '@sentry/node';
 import type { StackFrame } from '@sentry/types';
-import { basename, escapeStringForRegex, GLOBAL_OBJ, join, tracingContextFromHeaders } from '@sentry/utils';
+import { basename, escapeStringForRegex, GLOBAL_OBJ, join, logger, tracingContextFromHeaders } from '@sentry/utils';
 import type { RequestEvent } from '@sveltejs/kit';
 
 import { WRAPPED_MODULE_SUFFIX } from '../vite/autoInstrument';
@@ -68,3 +69,18 @@ export function rewriteFramesIteratee(frame: StackFrame): StackFrame {
 
   return frame;
 }
+
+/** Flush the event queue to ensure that events get sent to Sentry before the response is finished and the lambda ends */
+export async function flushIfServerless(): Promise<void> {
+  const platformSupportsStreaming = !process.env.LAMBDA_TASK_ROOT && !process.env.VERCEL;
+
+  if (!platformSupportsStreaming) {
+    try {
+      __DEBUG_BUILD__ && logger.log('Flushing events...');
+      await flush(2000);
+      __DEBUG_BUILD__ && logger.log('Done flushing events');
+    } catch (e) {
+      __DEBUG_BUILD__ && logger.log('Error while flushing events:\n', e);
+    }
+  }
+}
diff --git a/packages/sveltekit/test/server/load.test.ts b/packages/sveltekit/test/server/load.test.ts
