Skip to content

Commit

Permalink
feat(types): Add PolymorphicRequest type (#5744)
Browse files Browse the repository at this point in the history
This is a change which comes out of a code review[1] of #5703, moving the `CrossPlatformType` from `@sentry/utils` to `@sentry/types` and renaming it `PolymorphicRequest` (to match the existing `PolymorphicEvent`). Because it affects a number of files, I decided to pull it into its own PR, just to not confuse things in that one.

[1] #5703 (comment)
  • Loading branch information
lobsterkatie committed Sep 15, 2022
1 parent f1f03f2 commit 903addf
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 103 deletions.
3 changes: 2 additions & 1 deletion packages/node/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export type {
Breadcrumb,
BreadcrumbHint,
PolymorphicRequest,
Request,
SdkInfo,
Event,
Expand All @@ -15,7 +16,7 @@ export type {
Thread,
User,
} from '@sentry/types';
export type { AddRequestDataToEventOptions, CrossPlatformRequest } from '@sentry/utils';
export type { AddRequestDataToEventOptions } from '@sentry/utils';

export type { NodeOptions } from './types';

Expand Down
7 changes: 3 additions & 4 deletions packages/node/src/requestDataDeprecated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,19 @@

/* eslint-disable deprecation/deprecation */
/* eslint-disable @typescript-eslint/no-explicit-any */
import { Event, ExtractedNodeRequestData } from '@sentry/types';
import { Event, ExtractedNodeRequestData, PolymorphicRequest } from '@sentry/types';
import {
addRequestDataToEvent,
AddRequestDataToEventOptions,
CrossPlatformRequest,
extractRequestData as _extractRequestData,
} from '@sentry/utils';
import * as cookie from 'cookie';
import * as url from 'url';

/**
* @deprecated `Handlers.ExpressRequest` is deprecated and will be removed in v8. Use `CrossPlatformRequest` instead.
* @deprecated `Handlers.ExpressRequest` is deprecated and will be removed in v8. Use `PolymorphicRequest` instead.
*/
export type ExpressRequest = CrossPlatformRequest;
export type ExpressRequest = PolymorphicRequest;

/**
* Normalizes data from the request object, accounting for framework differences.
Expand Down
7 changes: 3 additions & 4 deletions packages/node/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
/* eslint-disable max-lines */
import { getCurrentHub, getIntegrationsToSetup, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
import { getMainCarrier, setHubOnCarrier } from '@sentry/hub';
import { Event, ExtractedNodeRequestData, SessionStatus, StackParser } from '@sentry/types';
import { Event, ExtractedNodeRequestData, PolymorphicRequest, SessionStatus, StackParser } from '@sentry/types';
import {
addRequestDataToEvent as _addRequestDataToEvent,
AddRequestDataToEventOptions,
createStackParser,
CrossPlatformRequest,
extractRequestData as _extractRequestData,
getGlobalObject,
logger,
Expand Down Expand Up @@ -294,7 +293,7 @@ function startSessionTracking(): void {
*/
export function addRequestDataToEvent(
event: Event,
req: CrossPlatformRequest,
req: PolymorphicRequest,
options?: Omit<AddRequestDataToEventOptions, 'deps'>,
): Event {
return _addRequestDataToEvent(event, req, {
Expand All @@ -318,7 +317,7 @@ export function addRequestDataToEvent(
* @returns An object containing normalized request data
*/
export function extractRequestData(
req: CrossPlatformRequest,
req: PolymorphicRequest,
options?: {
include?: string[];
},
Expand Down
21 changes: 10 additions & 11 deletions packages/node/test/requestdata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@

// TODO (v8 / #5257): Remove everything above

import { Event, TransactionSource, User } from '@sentry/types';
import { Event, PolymorphicRequest, TransactionSource, User } from '@sentry/types';
import {
addRequestDataToEvent,
AddRequestDataToEventOptions,
CrossPlatformRequest,
extractPathForTransaction,
extractRequestData as newExtractRequestData,
} from '@sentry/utils';
Expand Down Expand Up @@ -43,7 +42,7 @@ describe.each([parseRequest, addRequestDataToEvent])(
} else {
return [
event,
req as CrossPlatformRequest,
req as PolymorphicRequest,
{
include,
deps: {
Expand Down Expand Up @@ -277,7 +276,7 @@ describe.each([oldExtractRequestData, newExtractRequestData])(
return [req as ExpressRequest, include] as Parameters<typeof oldExtractRequestData>;
} else {
return [
req as CrossPlatformRequest,
req as PolymorphicRequest,
{
include,
deps: {
Expand Down Expand Up @@ -496,7 +495,7 @@ describe('extractPathForTransaction', () => {
baseUrl: '/api/users',
route: { path: '/:id/details' },
originalUrl: '/api/users/123/details',
} as CrossPlatformRequest,
} as PolymorphicRequest,
{ path: true, method: true },
'GET /api/users/:id/details',
'route' as TransactionSource,
Expand All @@ -508,7 +507,7 @@ describe('extractPathForTransaction', () => {
baseUrl: '/api/users',
route: { path: '/:id/details' },
originalUrl: '/api/users/123/details',
} as CrossPlatformRequest,
} as PolymorphicRequest,
{ path: true, method: false },
'/api/users/:id/details',
'route' as TransactionSource,
Expand All @@ -520,7 +519,7 @@ describe('extractPathForTransaction', () => {
baseUrl: '/api/users',
route: { path: '/:id/details' },
originalUrl: '/api/users/123/details',
} as CrossPlatformRequest,
} as PolymorphicRequest,
{ path: false, method: true },
'GET',
'route' as TransactionSource,
Expand All @@ -532,7 +531,7 @@ describe('extractPathForTransaction', () => {
baseUrl: '/api/users',
route: { path: '/:id/details' },
originalUrl: '/api/users/123/details',
} as CrossPlatformRequest,
} as PolymorphicRequest,
{ path: false, method: false },
'',
'route' as TransactionSource,
Expand All @@ -543,7 +542,7 @@ describe('extractPathForTransaction', () => {
method: 'get',
baseUrl: '/api/users',
originalUrl: '/api/users/123/details',
} as CrossPlatformRequest,
} as PolymorphicRequest,
{ path: true, method: true },
'GET /api/users/123/details',
'url' as TransactionSource,
Expand All @@ -552,7 +551,7 @@ describe('extractPathForTransaction', () => {
'%s',
(
_: string,
req: CrossPlatformRequest,
req: PolymorphicRequest,
options: { path?: boolean; method?: boolean },
expectedRoute: string,
expectedSource: TransactionSource,
Expand All @@ -570,7 +569,7 @@ describe('extractPathForTransaction', () => {
baseUrl: '/api/users',
route: { path: '/:id/details' },
originalUrl: '/api/users/123/details',
} as CrossPlatformRequest;
} as PolymorphicRequest;

const [route, source] = extractPathForTransaction(req, {
path: true,
Expand Down
14 changes: 4 additions & 10 deletions packages/tracing/src/integrations/node/express.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
/* eslint-disable max-lines */
import { Integration, Transaction } from '@sentry/types';
import {
CrossPlatformRequest,
extractPathForTransaction,
getNumberOfUrlSegments,
isRegExp,
logger,
} from '@sentry/utils';
import { Integration, PolymorphicRequest, Transaction } from '@sentry/types';
import { extractPathForTransaction, getNumberOfUrlSegments, isRegExp, logger } from '@sentry/utils';

type Method =
| 'all'
Expand Down Expand Up @@ -39,8 +33,8 @@ type Router = {
[method in Method]: (...args: any) => any; // eslint-disable-line @typescript-eslint/no-explicit-any
};

/* Extend the CrossPlatformRequest type with a patched parameter to build a reconstructed route */
type PatchedRequest = CrossPlatformRequest & { _reconstructedRoute?: string };
/* Extend the PolymorphicRequest type with a patched parameter to build a reconstructed route */
type PatchedRequest = PolymorphicRequest & { _reconstructedRoute?: string };

/* Types used for patching the express router prototype */
type ExpressRouter = Router & {
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export type { Mechanism } from './mechanism';
export type { ExtractedNodeRequestData, HttpHeaderValue, Primitive, WorkerLocation } from './misc';
export type { ClientOptions, Options } from './options';
export type { Package } from './package';
export type { PolymorphicEvent } from './polymorphics';
export type { PolymorphicEvent, PolymorphicRequest } from './polymorphics';
export type { QueryParams, Request } from './request';
export type { Runtime } from './runtime';
export type { CaptureContext, Scope, ScopeContext } from './scope';
Expand Down
67 changes: 67 additions & 0 deletions packages/types/src/polymorphics.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/**
* Event-like interface that's usable in browser and node.
*
* Note: Here we mean the kind of events handled by event listeners, not our `Event` type.
*
* Property availability taken from https://developer.mozilla.org/en-US/docs/Web/API/Event#browser_compatibility
*/
export interface PolymorphicEvent {
Expand All @@ -9,3 +11,68 @@ export interface PolymorphicEvent {
readonly target?: unknown;
readonly currentTarget?: unknown;
}

/** A `Request` type compatible with Node, Express, browser, etc., because everything is optional */
export type PolymorphicRequest = BaseRequest &
BrowserRequest &
NodeRequest &
ExpressRequest &
KoaRequest &
NextjsRequest;

type BaseRequest = {
method?: string;
url?: string;
};

type BrowserRequest = BaseRequest;

type NodeRequest = BaseRequest & {
headers?: {
[key: string]: string | string[] | undefined;
};
protocol?: string;
socket?: {
encrypted?: boolean;
remoteAddress?: string;
};
};

type KoaRequest = NodeRequest & {
host?: string;
hostname?: string;
ip?: string;
originalUrl?: string;
};

type NextjsRequest = NodeRequest & {
cookies?: {
[key: string]: string;
};
query?: {
[key: string]: any;
};
};

type ExpressRequest = NodeRequest & {
baseUrl?: string;
body?: string | { [key: string]: any };
host?: string;
hostname?: string;
ip?: string;
originalUrl?: string;
route?: {
path: string;
stack: [
{
name: string;
},
];
};
query?: {
[key: string]: any;
};
user?: {
[key: string]: any;
};
};

0 comments on commit 903addf

Please sign in to comment.