Skip to content

Commit 3410a08

Browse files
authored
ref(remix): Avoid unnecessary error wrapping HandleDocumentRequestFunction (#17680)
Noticed while working on #17679, this was actually incorrect here. `handleCallbackErrors` does not actually do anything, you need to still call the function you want to call in the error case! Actually we can just drop this as this error is handled by some other handler anyhow, I think, according to the tests!
1 parent c0f0375 commit 3410a08

File tree

2 files changed

+5
-59
lines changed

2 files changed

+5
-59
lines changed

packages/remix/src/server/errors.ts

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,4 @@
1-
import type {
2-
ActionFunction,
3-
ActionFunctionArgs,
4-
EntryContext,
5-
HandleDocumentRequestFunction,
6-
LoaderFunction,
7-
LoaderFunctionArgs,
8-
} from '@remix-run/node';
1+
import type { ActionFunction, ActionFunctionArgs, LoaderFunction, LoaderFunctionArgs } from '@remix-run/node';
92
import { isRouteErrorResponse } from '@remix-run/router';
103
import type { RequestEventData, Span } from '@sentry/core';
114
import {
@@ -79,37 +72,6 @@ export async function captureRemixServerException(err: unknown, name: string, re
7972
});
8073
}
8174

82-
/**
83-
* Wraps the original `HandleDocumentRequestFunction` with error handling.
84-
*
85-
* @param origDocumentRequestFunction The original `HandleDocumentRequestFunction`.
86-
* @param requestContext The request context.
87-
*
88-
* @returns The wrapped `HandleDocumentRequestFunction`.
89-
*/
90-
export function errorHandleDocumentRequestFunction(
91-
this: unknown,
92-
origDocumentRequestFunction: HandleDocumentRequestFunction,
93-
requestContext: {
94-
request: Request;
95-
responseStatusCode: number;
96-
responseHeaders: Headers;
97-
context: EntryContext;
98-
loadContext?: Record<string, unknown>;
99-
},
100-
): HandleDocumentRequestFunction {
101-
const { request, responseStatusCode, responseHeaders, context, loadContext } = requestContext;
102-
103-
return handleCallbackErrors(
104-
() => {
105-
return origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context, loadContext);
106-
},
107-
err => {
108-
throw err;
109-
},
110-
);
111-
}
112-
11375
/**
11476
* Wraps the original `DataFunction` with error handling.
11577
* This function also stores the form data keys if the action is being called.

packages/remix/src/server/instrumentServer.ts

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import type {
66
ActionFunctionArgs,
77
AppLoadContext,
88
CreateRequestHandlerFunction,
9-
EntryContext,
109
HandleDocumentRequestFunction,
1110
LoaderFunction,
1211
LoaderFunctionArgs,
@@ -39,7 +38,7 @@ import {
3938
import { DEBUG_BUILD } from '../utils/debug-build';
4039
import { createRoutes, getTransactionName } from '../utils/utils';
4140
import { extractData, isResponse, json } from '../utils/vendor/response';
42-
import { captureRemixServerException, errorHandleDataFunction, errorHandleDocumentRequestFunction } from './errors';
41+
import { captureRemixServerException, errorHandleDataFunction } from './errors';
4342

4443
type AppData = unknown;
4544
type RemixRequest = Parameters<RequestHandler>[0];
@@ -119,22 +118,7 @@ function getTraceAndBaggage(): {
119118

120119
function makeWrappedDocumentRequestFunction(instrumentTracing?: boolean) {
121120
return function (origDocumentRequestFunction: HandleDocumentRequestFunction): HandleDocumentRequestFunction {
122-
return async function (
123-
this: unknown,
124-
request: Request,
125-
responseStatusCode: number,
126-
responseHeaders: Headers,
127-
context: EntryContext,
128-
loadContext?: Record<string, unknown>,
129-
): Promise<Response> {
130-
const documentRequestContext = {
131-
request,
132-
responseStatusCode,
133-
responseHeaders,
134-
context,
135-
loadContext,
136-
};
137-
121+
return async function (this: unknown, request: Request, ...args: unknown[]): Promise<Response> {
138122
if (instrumentTracing) {
139123
const activeSpan = getActiveSpan();
140124
const rootSpan = activeSpan && getRootSpan(activeSpan);
@@ -155,11 +139,11 @@ function makeWrappedDocumentRequestFunction(instrumentTracing?: boolean) {
155139
},
156140
},
157141
() => {
158-
return errorHandleDocumentRequestFunction.call(this, origDocumentRequestFunction, documentRequestContext);
142+
return origDocumentRequestFunction.call(this, request, ...args);
159143
},
160144
);
161145
} else {
162-
return errorHandleDocumentRequestFunction.call(this, origDocumentRequestFunction, documentRequestContext);
146+
return origDocumentRequestFunction.call(this, request, ...args);
163147
}
164148
};
165149
};

0 commit comments

Comments
 (0)