Skip to content

Commit

Permalink
Add sanitization to defer errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@brophdawg11
brophdawg11 committed Mar 6, 2023
1 parent 4e2096a commit ef3042e
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 34 deletions.
23 changes: 12 additions & 11 deletions integration/error-sanitization-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,12 @@ test.describe("Error Sanitization", () => {

test.describe("serverMode=production", () => {
test.beforeAll(async () => {
fixture = await createFixture({
files: routeFiles,
});
fixture = await createFixture(
{
files: routeFiles,
},
ServerMode.Production
);
});

test("renders document without errors", async () => {
Expand Down Expand Up @@ -183,17 +186,16 @@ test.describe("Error Sanitization", () => {
expect(html).toMatch("Defer Route");
expect(html).toMatch("RESOLVED");
expect(html).not.toMatch("MESSAGE:");
expect(html).not.toMatch(/stack/i);
expect(html).not.toMatch(/"stack":/i);
});

test("sanitizes defer errors in document requests", async () => {
let response = await fixture.requestDocument("/defer?loader");
let html = await response.text();
expect(html).toMatch("Defer Error");
expect(html).not.toMatch("RESOLVED");
// TODO: is this expected or should we get "Unexpected Server Error" here?
expect(html).toMatch('{"message":"REJECTED"}');
expect(html).not.toMatch(/stack/i);
expect(html).toMatch('{"message":"Unexpected Server Error"}');
expect(html).not.toMatch(/"stack":/i);
});

test("returns data without errors", async () => {
Expand Down Expand Up @@ -291,17 +293,16 @@ test.describe("Error Sanitization", () => {
expect(html).toMatch("Defer Route");
expect(html).toMatch("RESOLVED");
expect(html).not.toMatch("MESSAGE:");
expect(html).not.toMatch(/stack/i);
expect(html).not.toMatch(/"stack":/i);
});

test("does not sanitize defer errors in document requests", async () => {
let response = await fixture.requestDocument("/defer?loader");
let html = await response.text();
expect(html).toMatch("Defer Error");
expect(html).not.toMatch("RESOLVED");
expect(html).toMatch('{"message":"REJECTED"}');
// TODO: I think we should be getting the stack here too?
expect(html).toMatch(/stack/i);
// TODO: Is our ServerMode.Development not making it into the build somehow?
expect(html).toMatch('{"message":"REJECTED","stack":"}');
});

test("returns data without errors", async () => {
Expand Down
24 changes: 19 additions & 5 deletions integration/helpers/create-fixture.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function json(value: JsonObject) {
}

export async function createFixture(init: FixtureInit, mode?: ServerMode) {
let projectDir = await createFixtureProject(init);
let projectDir = await createFixtureProject(init, mode);
let buildPath = path.resolve(projectDir, "build");
let app: ServerBuild = await import(buildPath);
let handler = createRequestHandler(app, mode || ServerMode.Production);
Expand Down Expand Up @@ -141,7 +141,8 @@ export async function createAppFixture(fixture: Fixture, mode?: ServerMode) {
////////////////////////////////////////////////////////////////////////////////

export async function createFixtureProject(
init: FixtureInit = {}
init: FixtureInit = {},
mode?: ServerMode
): Promise<string> {
let template = init.template ?? "node-template";
let integrationTemplateDir = path.join(__dirname, template);
Expand Down Expand Up @@ -193,17 +194,30 @@ export async function createFixtureProject(
}

await writeTestFiles(init, projectDir);
build(projectDir, init.buildStdio, init.sourcemap);
build(projectDir, init.buildStdio, init.sourcemap, mode);

return projectDir;
}

function build(projectDir: string, buildStdio?: Writable, sourcemap?: boolean) {
function build(
projectDir: string,
buildStdio?: Writable,
sourcemap?: boolean,
mode?: ServerMode
) {
let buildArgs = ["node_modules/@remix-run/dev/dist/cli.js", "build"];
if (sourcemap) {
buildArgs.push("--sourcemap");
}
let buildSpawn = spawnSync("node", buildArgs, { cwd: projectDir });

console.log("Building with NODE_ENV =", mode || ServerMode.Production);
let buildSpawn = spawnSync("node", buildArgs, {
cwd: projectDir,
env: {
...process.env,
NODE_ENV: mode || ServerMode.Production,
},
});

// These logs are helpful for debugging. Remove comments if needed.
// console.log("spawning @remix-run/dev/cli.js `build`:\n");
Expand Down
44 changes: 26 additions & 18 deletions packages/remix-react/components.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -815,8 +815,9 @@ export function Scripts(props: ScriptProps) {
: [
"__remixContext.p = function(v,e,p,x) {",
" if (typeof e !== 'undefined') {",
" x=new Error(e.message);",
process.env.NODE_ENV === "development" ? `x.stack=e.stack;` : "",
process.env.NODE_ENV === "development"
? " x=new Error(e.message);\n x.stack=e.stack;"
: ' x=new Error("Unexpected Server Error");\n x.stack=undefined;',
" p=Promise.reject(x);",
" } else {",
" p=Promise.resolve(v);",
Expand All @@ -835,8 +836,9 @@ export function Scripts(props: ScriptProps) {
"__remixContext.r = function(i,k,v,e,p,x) {",
" p = __remixContext.t[i][k];",
" if (typeof e !== 'undefined') {",
" x=new Error(e.message);",
process.env.NODE_ENV === "development" ? `x.stack=e.stack;` : "",
process.env.NODE_ENV === "development"
? " x=new Error(e.message);\n x.stack=e.stack;"
: ' x=new Error("Unexpected Server Error");\n x.stack=undefined;',
" p.e(x);",
" } else {",
" p.r(v);",
Expand Down Expand Up @@ -866,13 +868,16 @@ export function Scripts(props: ScriptProps) {
} else {
let trackedPromise = deferredData.data[key] as TrackedPromise;
if (typeof trackedPromise._error !== "undefined") {
let toSerialize: { message: string; stack?: string } = {
message: trackedPromise._error.message,
stack: undefined,
};
if (process.env.NODE_ENV === "development") {
toSerialize.stack = trackedPromise._error.stack;
}
let toSerialize: { message: string; stack?: string } =
process.env.NODE_ENV === "development"
? {
message: trackedPromise._error.message,
stack: trackedPromise._error.stack,
}
: {
message: "Unexpected Server Error",
stack: undefined,
};
return `${JSON.stringify(
key
)}:__remixContext.p(!1, ${escapeHtml(
Expand Down Expand Up @@ -1078,13 +1083,16 @@ function ErrorDeferredHydrationScript({
routeId: string;
}) {
let error = useAsyncError() as Error;
let toSerialize: { message: string; stack?: string } = {
message: error.message,
stack: undefined,
};
if (process.env.NODE_ENV === "development") {
toSerialize.stack = error.stack;
}
let toSerialize: { message: string; stack?: string } =
process.env.NODE_ENV === "development"
? {
message: error.message,
stack: error.stack,
}
: {
message: "Unexpected Server Error",
stack: undefined,
};

return (
<script
Expand Down

0 comments on commit ef3042e

Please sign in to comment.