Skip to content

Commit

Permalink
Merge pull request #9817 from getsentry/prepare-release/7.87.0
Browse files Browse the repository at this point in the history
Co-authored-by: Francesco Novy <[email protected]>
  • Loading branch information
Lms24 and mydea authored Dec 13, 2023
2 parents a063fbc + 28ba019 commit ac09686
Show file tree
Hide file tree
Showing 48 changed files with 531 additions and 185 deletions.
11 changes: 11 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,14 @@ updates:
prefix: ci
prefix-development: ci
include: scope
- package-ecosystem: 'npm'
directory: '/'
schedule:
interval: 'weekly'
allow:
- dependency-name: "@sentry/cli"
- dependency-name: "@sentry/vite-plugin"
commit-message:
prefix: feat
prefix-development: feat
include: scope
4 changes: 2 additions & 2 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
],
"deno.enablePaths": ["packages/deno/test"],
"editor.codeActionsOnSave": {
"source.organizeImports.biome": true,
"quickfix.biome": true
"source.organizeImports.biome": "explicit",
"quickfix.biome": "explicit"
},
"editor.defaultFormatter": "biomejs.biome"
}
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,18 @@

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

## 7.87.0

- feat: Add top level `getCurrentScope()` method (#9800)
- feat(replay): Bump `rrweb` to 2.5.0 (#9803)
- feat(replay): Capture hydration error breadcrumb (#9759)
- feat(types): Add profile envelope types (#9798)
- fix(astro): Avoid RegExp creation during route interpolation (#9815)
- fix(browser): Avoid importing from `./exports` (#9775)
- fix(nextjs): Catch rejecting flushes (#9811)
- fix(nextjs): Fix devserver CORS blockage when `assetPrefix` is defined (#9766)
- fix(node): Capture errors in tRPC middleware (#9782)

## 7.86.0

- feat(core): Use SDK_VERSION for hub API version (#9732)
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export {
getHubFromCarrier,
getCurrentHub,
getClient,
getCurrentScope,
Hub,
makeMain,
Scope,
Expand Down
94 changes: 80 additions & 14 deletions packages/astro/src/server/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@ import {
startSpan,
} from '@sentry/node';
import type { Hub, Span } from '@sentry/types';
import {
addNonEnumerableProperty,
objectify,
stripUrlQueryAndFragment,
tracingContextFromHeaders,
} from '@sentry/utils';
import { addNonEnumerableProperty, objectify, stripUrlQueryAndFragment } from '@sentry/utils';
import type { APIContext, MiddlewareResponseHandler } from 'astro';

import { getTracingMetaTags } from './meta';
Expand Down Expand Up @@ -64,7 +59,11 @@ type AstroLocalsWithSentry = Record<string, unknown> & {
};

export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseHandler = options => {
const handlerOptions = { trackClientIp: false, trackHeaders: false, ...options };
const handlerOptions = {
trackClientIp: false,
trackHeaders: false,
...options,
};

return async (ctx, next) => {
// if there is an active span, we know that this handle call is nested and hence
Expand Down Expand Up @@ -113,18 +112,19 @@ async function instrumentRequest(
}

try {
const interpolatedRoute = interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params);
// storing res in a variable instead of directly returning is necessary to
// invoke the catch block if next() throws
const res = await startSpan(
{
...traceCtx,
name: `${method} ${interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params)}`,
name: `${method} ${interpolatedRoute || ctx.url.pathname}`,
op: 'http.server',
origin: 'auto.http.astro',
status: 'ok',
metadata: {
...traceCtx?.metadata,
source: 'route',
source: interpolatedRoute ? 'route' : 'url',
},
data: {
method,
Expand Down Expand Up @@ -202,10 +202,76 @@ function addMetaTagToHead(htmlChunk: string, hub: Hub, span?: Span): string {
* Best we can do to get a route name instead of a raw URL.
*
* exported for testing
*
* @param rawUrlPathname - The raw URL pathname, e.g. '/users/123/details'
* @param params - The params object, e.g. `{ userId: '123' }`
*
* @returns The interpolated route, e.g. '/users/[userId]/details'
*/
export function interpolateRouteFromUrlAndParams(rawUrl: string, params: APIContext['params']): string {
return Object.entries(params).reduce((interpolateRoute, value) => {
const [paramId, paramValue] = value;
return interpolateRoute.replace(new RegExp(`(/|-)${paramValue}(/|-|$)`), `$1[${paramId}]$2`);
}, rawUrl);
export function interpolateRouteFromUrlAndParams(
rawUrlPathname: string,
params: APIContext['params'],
): string | undefined {
const decodedUrlPathname = tryDecodeUrl(rawUrlPathname);
if (!decodedUrlPathname) {
return undefined;
}

// Invert params map so that the param values are the keys
// differentiate between rest params spanning multiple url segments
// and normal, single-segment params.
const valuesToMultiSegmentParams: Record<string, string> = {};
const valuesToParams: Record<string, string> = {};
Object.entries(params).forEach(([key, value]) => {
if (!value) {
return;
}
if (value.includes('/')) {
valuesToMultiSegmentParams[value] = key;
return;
}
valuesToParams[value] = key;
});

function replaceWithParamName(segment: string): string {
const param = valuesToParams[segment];
if (param) {
return `[${param}]`;
}
return segment;
}

// before we match single-segment params, we first replace multi-segment params
const urlWithReplacedMultiSegmentParams = Object.keys(valuesToMultiSegmentParams).reduce((acc, key) => {
return acc.replace(key, `[${valuesToMultiSegmentParams[key]}]`);
}, decodedUrlPathname);

return urlWithReplacedMultiSegmentParams
.split('/')
.map(segment => {
if (!segment) {
return '';
}

if (valuesToParams[segment]) {
return replaceWithParamName(segment);
}

// astro permits multiple params in a single path segment, e.g. /[foo]-[bar]/
const segmentParts = segment.split('-');
if (segmentParts.length > 1) {
return segmentParts.map(part => replaceWithParamName(part)).join('-');
}

return segment;
})
.join('/');
}

function tryDecodeUrl(url: string): string | undefined {
try {
return decodeURI(url);
} catch {
return undefined;
}
}
60 changes: 60 additions & 0 deletions packages/astro/test/server/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,43 @@ describe('sentryMiddleware', () => {
expect(resultFromNext).toStrictEqual(nextResult);
});

it("sets source route if the url couldn't be decoded correctly", async () => {
const middleware = handleRequest();
const ctx = {
request: {
method: 'GET',
url: '/a%xx',
headers: new Headers(),
},
url: { pathname: 'a%xx', href: 'http://localhost:1234/a%xx' },
params: {},
};
const next = vi.fn(() => nextResult);

// @ts-expect-error, a partial ctx object is fine here
const resultFromNext = middleware(ctx, next);

expect(startSpanSpy).toHaveBeenCalledWith(
{
data: {
method: 'GET',
url: 'http://localhost:1234/a%xx',
},
metadata: {
source: 'url',
},
name: 'GET a%xx',
op: 'http.server',
origin: 'auto.http.astro',
status: 'ok',
},
expect.any(Function), // the `next` function
);

expect(next).toHaveBeenCalled();
expect(resultFromNext).toStrictEqual(nextResult);
});

it('throws and sends an error to sentry if `next()` throws', async () => {
const captureExceptionSpy = vi.spyOn(SentryNode, 'captureException');

Expand Down Expand Up @@ -299,15 +336,31 @@ describe('sentryMiddleware', () => {

describe('interpolateRouteFromUrlAndParams', () => {
it.each([
['/', {}, '/'],
['/foo/bar', {}, '/foo/bar'],
['/users/123', { id: '123' }, '/users/[id]'],
['/users/123', { id: '123', foo: 'bar' }, '/users/[id]'],
['/lang/en-US', { lang: 'en', region: 'US' }, '/lang/[lang]-[region]'],
['/lang/en-US/posts', { lang: 'en', region: 'US' }, '/lang/[lang]-[region]/posts'],
// edge cases that astro doesn't support
['/lang/-US', { region: 'US' }, '/lang/-[region]'],
['/lang/en-', { lang: 'en' }, '/lang/[lang]-'],
])('interpolates route from URL and params %s', (rawUrl, params, expectedRoute) => {
expect(interpolateRouteFromUrlAndParams(rawUrl, params)).toEqual(expectedRoute);
});

it.each([
['/(a+)+/aaaaaaaaa!', { id: '(a+)+', slug: 'aaaaaaaaa!' }, '/[id]/[slug]'],
['/([a-zA-Z]+)*/aaaaaaaaa!', { id: '([a-zA-Z]+)*', slug: 'aaaaaaaaa!' }, '/[id]/[slug]'],
['/(a|aa)+/aaaaaaaaa!', { id: '(a|aa)+', slug: 'aaaaaaaaa!' }, '/[id]/[slug]'],
['/(a|a?)+/aaaaaaaaa!', { id: '(a|a?)+', slug: 'aaaaaaaaa!' }, '/[id]/[slug]'],
// with URL encoding
['/(a%7Caa)+/aaaaaaaaa!', { id: '(a|aa)+', slug: 'aaaaaaaaa!' }, '/[id]/[slug]'],
['/(a%7Ca?)+/aaaaaaaaa!', { id: '(a|a?)+', slug: 'aaaaaaaaa!' }, '/[id]/[slug]'],
])('handles regex characters in param values correctly %s', (rawUrl, params, expectedRoute) => {
expect(interpolateRouteFromUrlAndParams(rawUrl, params)).toEqual(expectedRoute);
});

it('handles params across multiple URL segments in catchall routes', () => {
// Ideally, Astro would let us know that this is a catchall route so we can make the param [...catchall] but it doesn't
expect(
Expand All @@ -324,4 +377,11 @@ describe('interpolateRouteFromUrlAndParams', () => {
const expectedRoute = '/usernames/[name]';
expect(interpolateRouteFromUrlAndParams(rawUrl, params)).toEqual(expectedRoute);
});

it('handles set but undefined params', () => {
const rawUrl = '/usernames/user';
const params = { name: undefined, name2: '' };
const expectedRoute = '/usernames/user';
expect(interpolateRouteFromUrlAndParams(rawUrl, params)).toEqual(expectedRoute);
});
});
1 change: 1 addition & 0 deletions packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export {
getHubFromCarrier,
getCurrentHub,
getClient,
getCurrentScope,
Hub,
lastEventId,
makeMain,
Expand Down
3 changes: 1 addition & 2 deletions packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable max-lines */
import { getCurrentHub } from '@sentry/core';
import { getClient, getCurrentHub } from '@sentry/core';
import type {
Event as SentryEvent,
HandlerDataConsole,
Expand Down Expand Up @@ -31,7 +31,6 @@ import {
} from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import { getClient } from '../exports';
import { WINDOW } from '../helpers';

/** JSDoc */
Expand Down
4 changes: 2 additions & 2 deletions packages/browser/src/profiling/integration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { EventProcessor, Hub, Integration, Transaction } from '@sentry/types';
import type { EventEnvelope, EventProcessor, Hub, Integration, Transaction } from '@sentry/types';
import type { Profile } from '@sentry/types/src/profiling';
import { logger } from '@sentry/utils';

Expand Down Expand Up @@ -110,7 +110,7 @@ export class BrowserProfilingIntegration implements Integration {
}
}

addProfilesToEnvelope(envelope, profilesToAddToEnvelope);
addProfilesToEnvelope(envelope as EventEnvelope, profilesToAddToEnvelope);
});
} else {
logger.warn('[Profiling] Client does not support hooks, profiling will be disabled');
Expand Down
8 changes: 3 additions & 5 deletions packages/browser/src/profiling/utils.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
/* eslint-disable max-lines */

import { DEFAULT_ENVIRONMENT, getCurrentHub } from '@sentry/core';
import type { DebugImage, Envelope, Event, StackFrame, StackParser, Transaction } from '@sentry/types';
import { DEFAULT_ENVIRONMENT, getClient, getCurrentHub } from '@sentry/core';
import type { DebugImage, Envelope, Event, EventEnvelope, StackFrame, StackParser, Transaction } from '@sentry/types';
import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling';
import { GLOBAL_OBJ, browserPerformanceTimeOrigin, forEachEnvelopeItem, logger, uuid4 } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import { getClient } from '../exports';
import { WINDOW } from '../helpers';
import type { JSSelfProfile, JSSelfProfileStack, JSSelfProfiler, JSSelfProfilerConstructor } from './jsSelfProfiling';

Expand Down Expand Up @@ -301,13 +300,12 @@ export function convertJSSelfProfileToSampledFormat(input: JSSelfProfile): Profi
* Adds items to envelope if they are not already present - mutates the envelope.
* @param envelope
*/
export function addProfilesToEnvelope(envelope: Envelope, profiles: Profile[]): Envelope {
export function addProfilesToEnvelope(envelope: EventEnvelope, profiles: Profile[]): Envelope {
if (!profiles.length) {
return envelope;
}

for (const profile of profiles) {
// @ts-expect-error untyped envelope
envelope[1].push([{ type: 'profile' }, profile]);
}
return envelope;
Expand Down
1 change: 1 addition & 0 deletions packages/bun/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export {
getHubFromCarrier,
getCurrentHub,
getClient,
getCurrentScope,
Hub,
lastEventId,
makeMain,
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,3 +308,10 @@ export function lastEventId(): string | undefined {
export function getClient<C extends Client>(): C | undefined {
return getCurrentHub().getClient<C>();
}

/**
* Get the currently active scope.
*/
export function getCurrentScope(): Scope {
return getCurrentHub().getScope();
}
4 changes: 2 additions & 2 deletions packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class Hub implements HubInterface {
*/
public pushScope(): Scope {
// We want to clone the content of prev scope
const scope = Scope.clone(this.getScope());
const scope = this.getScope().clone();
this.getStack().push({
client: this.getClient(),
scope,
Expand Down Expand Up @@ -578,7 +578,7 @@ export function ensureHubOnCarrier(carrier: Carrier, parent: Hub = getGlobalHub(
// If there's no hub on current domain, or it's an old API, assign a new one
if (!hasHubOnCarrier(carrier) || getHubFromCarrier(carrier).isOlderThan(API_VERSION)) {
const globalHubTopStack = parent.getStackTop();
setHubOnCarrier(carrier, new Hub(globalHubTopStack.client, Scope.clone(globalHubTopStack.scope)));
setHubOnCarrier(carrier, new Hub(globalHubTopStack.client, globalHubTopStack.scope.clone()));
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export {
setUser,
withScope,
getClient,
getCurrentScope,
} from './exports';
export {
getCurrentHub,
Expand Down
Loading

0 comments on commit ac09686

Please sign in to comment.