Skip to content

meta(changelog): Update changelog for 7.51.2 #8076

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 6 commits into from
May 8, 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
1 change: 1 addition & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ jobs:
job_node_integration_tests,
job_browser_playwright_tests,
job_browser_integration_tests,
job_browser_loader_tests,
job_remix_integration_tests,
job_e2e_tests,
]
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

## 7.51.2

- fix(nextjs): Continue traces in data fetchers when there is an already active transaction on the hub (#8073)
- fix(sveltekit): Avoid creating the Sentry Vite plugin in dev mode (#8065)

## 7.51.1

- feat(replay): Add event to capture options on checkouts (#8011)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,13 @@
"$schema": "../../test-recipe-schema.json",
"testApplicationName": "create-react-app",
"buildCommand": "pnpm install && pnpm build",
"tests": []
"tests": [],
"canaryVersions": [
{
"dependencyOverrides": {
"react": "canary",
"react-dom": "canary"
}
}
]
}
25 changes: 13 additions & 12 deletions packages/nextjs/src/server/utils/wrapperUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
runWithAsyncContext,
startTransaction,
} from '@sentry/core';
import type { Transaction } from '@sentry/types';
import type { Span, Transaction } from '@sentry/types';
import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils';
import type { IncomingMessage, ServerResponse } from 'http';

Expand Down Expand Up @@ -82,7 +82,8 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
return async function (this: unknown, ...args: Parameters<F>): Promise<ReturnType<F>> {
return runWithAsyncContext(async () => {
const hub = getCurrentHub();
let requestTransaction: Transaction | undefined = getTransactionFromRequest(req);
const scope = hub.getScope();
const previousSpan: Span | undefined = getTransactionFromRequest(req) ?? scope.getSpan();
let dataFetcherSpan;

const sentryTraceHeader = req.headers['sentry-trace'];
Expand All @@ -93,7 +94,8 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(rawBaggageString);

if (platformSupportsStreaming()) {
if (requestTransaction === undefined) {
let spanToContinue: Span;
if (previousSpan === undefined) {
const newTransaction = startTransaction(
{
op: 'http.server',
Expand All @@ -109,8 +111,6 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
{ request: req },
);

requestTransaction = newTransaction;

if (platformSupportsStreaming()) {
// On platforms that don't support streaming, doing things after res.end() is unreliable.
autoEndTransactionOnResponseEnd(newTransaction, res);
Expand All @@ -119,9 +119,12 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
// Link the transaction and the request together, so that when we would normally only have access to one, it's
// still possible to grab the other.
setTransactionOnRequest(newTransaction, req);
spanToContinue = newTransaction;
} else {
spanToContinue = previousSpan;
}

dataFetcherSpan = requestTransaction.startChild({
dataFetcherSpan = spanToContinue.startChild({
op: 'function.nextjs',
description: `${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`,
status: 'ok',
Expand All @@ -140,22 +143,20 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
});
}

const currentScope = hub.getScope();
if (currentScope) {
currentScope.setSpan(dataFetcherSpan);
currentScope.setSDKProcessingMetadata({ request: req });
}
scope.setSpan(dataFetcherSpan);
scope.setSDKProcessingMetadata({ request: req });

try {
return await origDataFetcher.apply(this, args);
} catch (e) {
// Since we finish the span before the error can bubble up and trigger the handlers in `registerErrorInstrumentation`
// that set the transaction status, we need to manually set the status of the span & transaction
dataFetcherSpan.setStatus('internal_error');
requestTransaction?.setStatus('internal_error');
previousSpan?.setStatus('internal_error');
throw e;
} finally {
dataFetcherSpan.finish();
scope.setSpan(previousSpan);
if (!platformSupportsStreaming()) {
await flushQueue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetI
const { req, res } = context.ctx;

const errorWrappedAppGetInitialProps = withErrorInstrumentation(wrappingTarget);
const options = getCurrentHub().getClient()?.getOptions();
const hub = getCurrentHub();
const options = hub.getClient()?.getOptions();

// Generally we can assume that `req` and `res` are always defined on the server:
// https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
Expand All @@ -51,7 +52,7 @@ export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetI
};
} = await tracedGetInitialProps.apply(thisArg, args);

const requestTransaction = getTransactionFromRequest(req);
const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction();

// Per definition, `pageProps` is not optional, however an increased amount of users doesn't seem to call
// `App.getInitialProps(appContext)` in their custom `_app` pages which is required as per
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ export function wrapErrorGetInitialPropsWithSentry(
const { req, res } = context;

const errorWrappedGetInitialProps = withErrorInstrumentation(wrappingTarget);
const options = getCurrentHub().getClient()?.getOptions();
const hub = getCurrentHub();
const options = hub.getClient()?.getOptions();

// Generally we can assume that `req` and `res` are always defined on the server:
// https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
Expand All @@ -52,7 +53,7 @@ export function wrapErrorGetInitialPropsWithSentry(
_sentryBaggage?: string;
} = await tracedGetInitialProps.apply(thisArg, args);

const requestTransaction = getTransactionFromRequest(req);
const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction();
if (requestTransaction) {
errorGetInitialProps._sentryTraceData = requestTransaction.toTraceparent();

Expand Down
5 changes: 3 additions & 2 deletions packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialPro
const { req, res } = context;

const errorWrappedGetInitialProps = withErrorInstrumentation(wrappingTarget);
const options = getCurrentHub().getClient()?.getOptions();
const hub = getCurrentHub();
const options = hub.getClient()?.getOptions();

// Generally we can assume that `req` and `res` are always defined on the server:
// https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
Expand All @@ -48,7 +49,7 @@ export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialPro
_sentryBaggage?: string;
} = await tracedGetInitialProps.apply(thisArg, args);

const requestTransaction = getTransactionFromRequest(req);
const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction();
if (requestTransaction) {
initialProps._sentryTraceData = requestTransaction.toTraceparent();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ export function wrapGetServerSidePropsWithSentry(
const { req, res } = context;

const errorWrappedGetServerSideProps = withErrorInstrumentation(wrappingTarget);
const options = getCurrentHub().getClient()?.getOptions();
const hub = getCurrentHub();
const options = hub.getClient()?.getOptions();

if (hasTracingEnabled() && options?.instrumenter === 'sentry') {
const tracedGetServerSideProps = withTracedServerSideDataFetcher(errorWrappedGetServerSideProps, req, res, {
Expand All @@ -45,7 +46,7 @@ export function wrapGetServerSidePropsWithSentry(
>);

if (serverSideProps && 'props' in serverSideProps) {
const requestTransaction = getTransactionFromRequest(req);
const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction();
if (requestTransaction) {
serverSideProps.props._sentryTraceData = requestTransaction.toTraceparent();

Expand Down
16 changes: 11 additions & 5 deletions packages/nextjs/test/config/wrappers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { IncomingMessage, ServerResponse } from 'http';
import { wrapGetInitialPropsWithSentry, wrapGetServerSidePropsWithSentry } from '../../src/server';

const startTransactionSpy = jest.spyOn(SentryCore, 'startTransaction');
const originalGetCurrentHub = jest.requireActual('@sentry/node').getCurrentHub;

// The wrap* functions require the hub to have tracing extensions. This is normally called by the NodeClient
// constructor but the client isn't used in these tests.
Expand All @@ -21,13 +22,18 @@ describe('data-fetching function wrappers', () => {
req = { headers: {}, url: 'http://dogs.are.great/tricks/kangaroo' } as IncomingMessage;
res = { end: jest.fn() } as unknown as ServerResponse;

jest.spyOn(SentryCore, 'hasTracingEnabled').mockReturnValueOnce(true);
jest.spyOn(SentryNode, 'getCurrentHub').mockReturnValueOnce({
getClient: () =>
jest.spyOn(SentryCore, 'hasTracingEnabled').mockReturnValue(true);
jest.spyOn(SentryNode, 'getCurrentHub').mockImplementation(() => {
const hub = originalGetCurrentHub();

hub.getClient = () =>
({
getOptions: () => ({ instrumenter: 'sentry' }),
} as any),
} as any);
getDsn: () => {},
} as any);

return hub;
});
});

afterEach(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/sveltekit/src/vite/sentryVitePlugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {}
);
}

if (mergedOptions.autoUploadSourceMaps) {
if (mergedOptions.autoUploadSourceMaps && process.env.NODE_ENV !== 'development') {
const pluginOptions = {
...mergedOptions.sourceMapsUploadOptions,
debug: mergedOptions.debug, // override the plugin's debug flag with the one from the top-level options
Expand Down
13 changes: 13 additions & 0 deletions packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,19 @@ describe('sentryVite()', () => {
expect(plugins).toHaveLength(1);
});

it("doesn't return the custom sentry source maps plugin if `NODE_ENV` is development", async () => {
const previousEnv = process.env.NODE_ENV;

process.env.NODE_ENV = 'development';
const plugins = await sentrySvelteKit({ autoUploadSourceMaps: true, autoInstrument: true });
const instrumentPlugin = plugins[0];

expect(plugins).toHaveLength(1);
expect(instrumentPlugin.name).toEqual('sentry-auto-instrumentation');

process.env.NODE_ENV = previousEnv;
});

it("doesn't return the auto instrument plugin if autoInstrument is `false`", async () => {
const plugins = await sentrySvelteKit({ autoInstrument: false });
expect(plugins).toHaveLength(1);
Expand Down