-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(remix): Add Remix server SDK #5269
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/* eslint-disable import/export */ | ||
import { configureScope, getCurrentHub, init as nodeInit } from '@sentry/node'; | ||
import { logger } from '@sentry/utils'; | ||
|
||
import { instrumentServer } from './utils/instrumentServer'; | ||
import { buildMetadata } from './utils/metadata'; | ||
import { RemixOptions } from './utils/remixOptions'; | ||
|
||
export { ErrorBoundary, withErrorBoundary } from '@sentry/react'; | ||
export { remixRouterInstrumentation, withSentryRouteTracing } from './performance/client'; | ||
export { BrowserTracing, Integrations } from '@sentry/tracing'; | ||
export * from '@sentry/node'; | ||
|
||
function sdkAlreadyInitialized(): boolean { | ||
const hub = getCurrentHub(); | ||
return !!hub.getClient(); | ||
} | ||
|
||
/** Initializes Sentry Remix SDK on Node. */ | ||
export function init(options: RemixOptions): void { | ||
buildMetadata(options, ['remix', 'node']); | ||
|
||
if (sdkAlreadyInitialized()) { | ||
__DEBUG_BUILD__ && logger.log('SDK already initialized'); | ||
|
||
return; | ||
AbhiPrasad marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
instrumentServer(); | ||
|
||
nodeInit(options); | ||
|
||
configureScope(scope => { | ||
scope.setTag('runtime', 'node'); | ||
}); | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,197 @@ | ||
import { captureException, configureScope, getCurrentHub, startTransaction } from '@sentry/node'; | ||
import { getActiveTransaction } from '@sentry/tracing'; | ||
import { addExceptionMechanism, fill, loadModule, logger } from '@sentry/utils'; | ||
|
||
// Types vendored from @remix-run/server-runtime@1.6.0: | ||
// https://github.com/remix-run/remix/blob/f3691d51027b93caa3fd2cdfe146d7b62a6eb8f2/packages/remix-server-runtime/server.ts | ||
type AppLoadContext = unknown; | ||
type AppData = unknown; | ||
type RequestHandler = (request: Request, loadContext?: AppLoadContext) => Promise<Response>; | ||
type CreateRequestHandlerFunction = (build: ServerBuild, mode?: string) => RequestHandler; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these all vendored in? Can we link to the code + git sha where we did this? |
||
type ServerRouteManifest = RouteManifest<Omit<ServerRoute, 'children'>>; | ||
type Params<Key extends string = string> = { | ||
readonly [key in Key]: string | undefined; | ||
}; | ||
|
||
interface Route { | ||
index?: boolean; | ||
caseSensitive?: boolean; | ||
id: string; | ||
parentId?: string; | ||
path?: string; | ||
} | ||
|
||
interface ServerRouteModule { | ||
action?: DataFunction; | ||
headers?: unknown; | ||
loader?: DataFunction; | ||
} | ||
|
||
interface ServerRoute extends Route { | ||
children: ServerRoute[]; | ||
module: ServerRouteModule; | ||
} | ||
|
||
interface RouteManifest<Route> { | ||
[routeId: string]: Route; | ||
} | ||
|
||
interface ServerBuild { | ||
entry: { | ||
module: ServerEntryModule; | ||
}; | ||
routes: ServerRouteManifest; | ||
assets: unknown; | ||
} | ||
|
||
interface HandleDocumentRequestFunction { | ||
(request: Request, responseStatusCode: number, responseHeaders: Headers, context: Record<symbol, unknown>): | ||
| Promise<Response> | ||
| Response; | ||
} | ||
|
||
interface HandleDataRequestFunction { | ||
(response: Response, args: DataFunctionArgs): Promise<Response> | Response; | ||
} | ||
|
||
interface ServerEntryModule { | ||
default: HandleDocumentRequestFunction; | ||
handleDataRequest?: HandleDataRequestFunction; | ||
} | ||
|
||
interface DataFunctionArgs { | ||
request: Request; | ||
context: AppLoadContext; | ||
params: Params; | ||
} | ||
|
||
interface DataFunction { | ||
(args: DataFunctionArgs): Promise<Response> | Response | Promise<AppData> | AppData; | ||
} | ||
|
||
function makeWrappedDataFunction(origFn: DataFunction, name: 'action' | 'loader'): DataFunction { | ||
return async function (this: unknown, args: DataFunctionArgs): Promise<Response | AppData> { | ||
let res: Response | AppData; | ||
const activeTransaction = getActiveTransaction(); | ||
const currentScope = getCurrentHub().getScope(); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of using optional chaining, let's just early return if there is no |
||
if (!activeTransaction || !currentScope) { | ||
return origFn.call(this, args); | ||
} | ||
|
||
try { | ||
const span = activeTransaction.startChild({ | ||
op: `remix.server.${name}`, | ||
description: activeTransaction.name, | ||
tags: { | ||
name, | ||
}, | ||
}); | ||
|
||
if (span) { | ||
// Assign data function to hub to be able to see `db` transactions (if any) as children. | ||
currentScope.setSpan(span); | ||
} | ||
|
||
res = await origFn.call(this, args); | ||
|
||
currentScope.setSpan(activeTransaction); | ||
span.finish(); | ||
} catch (err) { | ||
configureScope(scope => { | ||
scope.addEventProcessor(event => { | ||
addExceptionMechanism(event, { | ||
type: 'instrument', | ||
handled: true, | ||
data: { | ||
function: name, | ||
}, | ||
}); | ||
|
||
return event; | ||
}); | ||
}); | ||
|
||
captureException(err); | ||
|
||
// Rethrow for other handlers | ||
throw err; | ||
} | ||
|
||
return res; | ||
}; | ||
} | ||
|
||
function makeWrappedAction(origAction: DataFunction): DataFunction { | ||
return makeWrappedDataFunction(origAction, 'action'); | ||
} | ||
|
||
function makeWrappedLoader(origAction: DataFunction): DataFunction { | ||
return makeWrappedDataFunction(origAction, 'loader'); | ||
} | ||
|
||
function wrapRequestHandler(origRequestHandler: RequestHandler): RequestHandler { | ||
return async function (this: unknown, request: Request, loadContext?: unknown): Promise<Response> { | ||
const currentScope = getCurrentHub().getScope(); | ||
const transaction = startTransaction({ | ||
name: request.url, | ||
op: 'http.server', | ||
tags: { | ||
method: request.method, | ||
}, | ||
}); | ||
|
||
if (transaction) { | ||
currentScope?.setSpan(transaction); | ||
} | ||
|
||
const res = (await origRequestHandler.call(this, request, loadContext)) as Response; | ||
|
||
transaction?.setHttpStatus(res.status); | ||
transaction?.finish(); | ||
|
||
return res; | ||
}; | ||
} | ||
|
||
function makeWrappedCreateRequestHandler( | ||
origCreateRequestHandler: CreateRequestHandlerFunction, | ||
): CreateRequestHandlerFunction { | ||
return function (this: unknown, build: ServerBuild, mode: string | undefined): RequestHandler { | ||
const routes: ServerRouteManifest = {}; | ||
|
||
for (const [id, route] of Object.entries(build.routes)) { | ||
const wrappedRoute = { ...route, module: { ...route.module } }; | ||
|
||
if (wrappedRoute.module.action) { | ||
fill(wrappedRoute.module, 'action', makeWrappedAction); | ||
} | ||
|
||
if (wrappedRoute.module.loader) { | ||
fill(wrappedRoute.module, 'loader', makeWrappedLoader); | ||
} | ||
|
||
routes[id] = wrappedRoute; | ||
} | ||
|
||
const requestHandler = origCreateRequestHandler.call(this, { ...build, routes }, mode); | ||
|
||
return wrapRequestHandler(requestHandler); | ||
}; | ||
} | ||
|
||
/** | ||
* Monkey-patch Remix's `createRequestHandler` from `@remix-run/server-runtime` | ||
* which Remix Adapters (https://remix.run/docs/en/v1/api/remix) use underneath. | ||
*/ | ||
export function instrumentServer(): void { | ||
const pkg = loadModule<{ createRequestHandler: CreateRequestHandlerFunction }>('@remix-run/server-runtime'); | ||
|
||
if (!pkg) { | ||
__DEBUG_BUILD__ && logger.warn('Remix SDK was unable to require `@remix-run/server-runtime` package.'); | ||
|
||
return; | ||
} | ||
|
||
fill(pkg, 'createRequestHandler', makeWrappedCreateRequestHandler); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import { NodeOptions } from '@sentry/node'; | ||
import { BrowserOptions } from '@sentry/react'; | ||
import { Options } from '@sentry/types'; | ||
|
||
export type RemixOptions = Options | BrowserOptions; | ||
export type RemixOptions = Options | BrowserOptions | NodeOptions; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
import * as SentryNode from '@sentry/node'; | ||
import { getCurrentHub } from '@sentry/node'; | ||
import { getGlobalObject } from '@sentry/utils'; | ||
|
||
import { init } from '../src/index.server'; | ||
|
||
const global = getGlobalObject(); | ||
|
||
const nodeInit = jest.spyOn(SentryNode, 'init'); | ||
|
||
describe('Server init()', () => { | ||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
global.__SENTRY__.hub = undefined; | ||
}); | ||
|
||
it('inits the Node SDK', () => { | ||
expect(nodeInit).toHaveBeenCalledTimes(0); | ||
init({}); | ||
expect(nodeInit).toHaveBeenCalledTimes(1); | ||
expect(nodeInit).toHaveBeenLastCalledWith( | ||
expect.objectContaining({ | ||
_metadata: { | ||
sdk: { | ||
name: 'sentry.javascript.remix', | ||
version: expect.any(String), | ||
packages: [ | ||
{ | ||
name: 'npm:@sentry/remix', | ||
version: expect.any(String), | ||
}, | ||
{ | ||
name: 'npm:@sentry/node', | ||
version: expect.any(String), | ||
}, | ||
], | ||
}, | ||
}, | ||
}), | ||
); | ||
}); | ||
|
||
it("doesn't reinitialize the node SDK if already initialized", () => { | ||
expect(nodeInit).toHaveBeenCalledTimes(0); | ||
init({}); | ||
expect(nodeInit).toHaveBeenCalledTimes(1); | ||
init({}); | ||
expect(nodeInit).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
it('sets runtime on scope', () => { | ||
const currentScope = getCurrentHub().getScope(); | ||
|
||
// @ts-ignore need access to protected _tags attribute | ||
expect(currentScope._tags).toEqual({}); | ||
|
||
init({}); | ||
|
||
// @ts-ignore need access to protected _tags attribute | ||
expect(currentScope._tags).toEqual({ runtime: 'node' }); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do these have side effects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took that from Next.JS SDK.
As I understand, we need to export browser / react stuff from
index.server.ts
to make TypeScript happy on compile time, but it's not guaranteed that it'll be used on runtime. (index.client.ts
is used instead on the demo project).So my guess is that flagging
index.server.*
as side effects is to prevent a pre-bundler that may fail TS compilation.