Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 33 additions & 23 deletions packages/sveltekit/src/server/handle.ts
Original file line number Diff line number Diff line change
@@ -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 = {
/**
Expand Down Expand Up @@ -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();
}
}
6 changes: 5 additions & 1 deletion packages/sveltekit/src/server/handleError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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);
}
Expand All @@ -36,6 +38,8 @@ export function handleErrorWithSentry(handleError: HandleServerError = defaultEr
return scope;
});

await flushIfServerless();

return handleError(input);
};
}
Expand Down
28 changes: 22 additions & 6 deletions packages/sveltekit/src/server/load.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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();
}
},
});
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
},
});
}
18 changes: 17 additions & 1 deletion packages/sveltekit/src/server/utils.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
}
}
}
Loading