Skip to content

Commit abc320b

Browse files
nams1570N2D4
andauthored
[Refactor] [Fix] Email Rendering Pipeline Refactor, Error Handling, and Bug Fixes (#1140)
### Context We noticed some errors pop up on sentry related to email rendering. These errors seem to have been triggered by the same issue, and could be categorized as follows: 1. Sanity test mismatch, even when the errors from freestyle and vercel sandbox were broadly similar. This occurred due to stack traces differing in different execution environments. 2. Rendering errors from freestyle and vercel sandbox caused by the theme not being imported/ empty theme component. Upon investigation, this occurred because hitting save on the email themes page with an invalid theme (ex: deleting the `export` keyword, or renaming the `EmailTheme` component) still triggers `bundleAndExecute` with the invalid themes. This will obviously fail and cause the errors to be logged, however there is no cause for concern here because the error is returned and the save is denied because an error is returned. It's more of a matter of noisy error logs and too strict sanity test comparisons. Beyond that, `js-execution` is a little opaque and hard to understand, and this can mask errors in logic. We also noticed a new issue: manually throwing an error in the email theme code editor, and then trying to save was actually successful. This was because the version of `react-email/components` we were using had faulty error handling, and fell back to client side rendering, masking the error. This wasn't caught by our `try-catch` safeguards because it was a render time issue that was masked. More specifically, this was what `react-email` was doing: `Switched to client rendering because the server rendering errored`. ### Summary of Changes We loosen the sanity test comparison between engine execution results in case of errors. We then refactor the `js-execution` and `email-rendering` files to read better, and to only `captureError` when a service is down, but not for runtime errors in the user submitted code. To deal with the other bug, we bumped `react-email/components` to the latest version. However, doing so exposed a gap between real `freestyle` and our `freestyle-mock`: with the mock, the errors that were now raised were treated as uncaught exceptions, crashing the mock server. Consequently, we switched to using `node` over `bun`. We also expanded test coverage to account for different error paths. Co-authored-by: Konstantin Wohlwend <n2d4xc@gmail.com>
1 parent 31b8d80 commit abc320b

10 files changed

Lines changed: 941 additions & 219 deletions

File tree

apps/backend/src/lib/email-rendering.tsx

Lines changed: 6 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
import { executeJavascript } from '@/lib/js-execution';
1+
import { executeJavascript, type ExecuteResult } from '@/lib/js-execution';
22
import { emptyEmailTheme } from '@stackframe/stack-shared/dist/helpers/emails';
3-
import { getNodeEnvironment } from '@stackframe/stack-shared/dist/utils/env';
4-
import { captureError, StackAssertionError } from '@stackframe/stack-shared/dist/utils/errors';
3+
import { StackAssertionError } from '@stackframe/stack-shared/dist/utils/errors';
54
import { bundleJavaScript } from '@stackframe/stack-shared/dist/utils/esbuild';
65
import { get, has } from '@stackframe/stack-shared/dist/utils/objects';
76
import { Result } from "@stackframe/stack-shared/dist/utils/results";
@@ -50,7 +49,7 @@ export function createTemplateComponentFromHtml(html: string) {
5049
const nodeModules = {
5150
"react-dom": "19.1.1",
5251
"react": "19.1.1",
53-
"@react-email/components": "0.1.1",
52+
"@react-email/components": "1.0.6",
5453
"arktype": "2.1.20",
5554
};
5655

@@ -70,12 +69,9 @@ const entryJs = deindent`
7069
`;
7170

7271
type EmailRenderResult = { html: string, text: string, subject?: string, notificationCategory?: string };
73-
type ExecuteResult =
74-
| { status: "ok", data: unknown }
75-
| { status: "error", error: unknown };
7672

7773
async function bundleAndExecute<T>(
78-
files: Record<string, string> & { '/entry.js': string }
74+
files: Record<string, string> & { '/entry.js': string },
7975
): Promise<Result<T, string>> {
8076
const bundle = await bundleJavaScript(files, {
8177
keepAsImports: ['arktype', 'react', 'react/jsx-runtime', '@react-email/components'],
@@ -87,40 +83,11 @@ async function bundleAndExecute<T>(
8783
return Result.error(bundle.error);
8884
}
8985

90-
if (["development", "test"].includes(getNodeEnvironment())) {
91-
const executeResult = await executeJavascript(bundle.data, { nodeModules, engine: 'freestyle' }) as ExecuteResult;
92-
if (executeResult.status === "error") {
93-
return Result.error(JSON.stringify(executeResult.error));
94-
}
95-
return Result.ok(executeResult.data as T);
96-
}
97-
98-
const executeResult = await executeJavascript(bundle.data, { nodeModules }) as ExecuteResult;
86+
const executeResult: ExecuteResult = await executeJavascript(bundle.data, { nodeModules });
9987

10088
if (executeResult.status === "error") {
101-
const vercelResult = await executeJavascript(bundle.data, { nodeModules, engine: 'vercel-sandbox' }) as ExecuteResult;
102-
if (vercelResult.status === "error") {
103-
captureError("email-rendering-freestyle-and-vercel-runtime-error", new StackAssertionError(
104-
"Email rendering failed with both freestyle and vercel-sandbox engines",
105-
{
106-
freestyleError: executeResult.error,
107-
vercelError: vercelResult.error,
108-
innerCode: bundle.data,
109-
innerOptions: [
110-
{ nodeModules, engine: 'freestyle' },
111-
{ nodeModules, engine: 'vercel-sandbox' },
112-
],
113-
},
114-
));
115-
return Result.error(JSON.stringify(vercelResult.error));
116-
}
117-
captureError("email-rendering-freestyle-runtime-error", new StackAssertionError(
118-
"Email rendering failed with freestyle but succeeded with vercel-sandbox",
119-
{ freestyleError: executeResult.error, innerCode: bundle.data, innerOptions: { nodeModules, engine: 'freestyle' } }
120-
));
121-
return Result.ok(vercelResult.data as T);
89+
return Result.error(JSON.stringify(executeResult.error));
12290
}
123-
12491
return Result.ok(executeResult.data as T);
12592
}
12693

apps/backend/src/lib/js-execution.tsx

Lines changed: 75 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,21 @@ import { Freestyle as FreestyleClient } from 'freestyle-sandboxes';
88

99
export type ExecuteJavascriptOptions = {
1010
nodeModules?: Record<string, string>,
11-
engine?: 'freestyle' | 'vercel-sandbox',
1211
};
1312

13+
export type ExecuteResult =
14+
| { status: "ok", data: unknown }
15+
| { status: "error", error: { message: string, stack?: string, cause?: unknown } };
16+
1417
type JsEngine = {
1518
name: string,
16-
execute: (code: string, options: ExecuteJavascriptOptions) => Promise<unknown>,
19+
execute: (code: string, options: ExecuteJavascriptOptions) => Promise<ExecuteResult>,
1720
};
1821

1922
function createFreestyleEngine(): JsEngine {
2023
return {
2124
name: 'freestyle',
22-
execute: async (code: string, options: ExecuteJavascriptOptions): Promise<unknown> => {
25+
execute: async (code: string, options: ExecuteJavascriptOptions): Promise<ExecuteResult> => {
2326
const apiKey = getEnvVariable("STACK_FREESTYLE_API_KEY");
2427
let baseUrl = getEnvVariable("STACK_FREESTYLE_API_ENDPOINT", "") || undefined;
2528

@@ -45,15 +48,15 @@ function createFreestyleEngine(): JsEngine {
4548
throw new StackAssertionError("Freestyle execution returned undefined result", { response, innerCode: code, innerOptions: options });
4649
}
4750

48-
return response.result;
51+
return response.result as ExecuteResult;
4952
},
5053
};
5154
}
5255

5356
function createVercelSandboxEngine(): JsEngine {
5457
return {
5558
name: 'vercel-sandbox',
56-
execute: async (code: string, options: ExecuteJavascriptOptions): Promise<unknown> => {
59+
execute: async (code: string, options: ExecuteJavascriptOptions): Promise<ExecuteResult> => {
5760
const teamId = getEnvVariable("STACK_VERCEL_SANDBOX_TEAM_ID", "");
5861
const projectId = getEnvVariable("STACK_VERCEL_SANDBOX_PROJECT_ID", "");
5962
const token = getEnvVariable("STACK_VERCEL_SANDBOX_TOKEN", "");
@@ -122,51 +125,66 @@ const engineMap = new Map<string, JsEngine>([
122125
['vercel-sandbox', createVercelSandboxEngine()],
123126
]);
124127

125-
const engines: JsEngine[] = Array.from(engineMap.values());
126-
127128
/**
128129
* Executes the given code with the given options. Returns the result of the code execution
129130
* if it is JSON-serializable. Has undefined behavior if it is not JSON-serializable or if
130131
* the code throws an error.
131132
*/
132-
export async function executeJavascript(code: string, options: ExecuteJavascriptOptions = {}): Promise<unknown> {
133+
export async function executeJavascript(code: string, options: ExecuteJavascriptOptions = {}): Promise<ExecuteResult> {
133134
return await traceSpan({
134135
description: 'js-execution.executeJavascript',
135136
attributes: {
136137
'js-execution.code.length': code.length.toString(),
137138
'js-execution.nodeModules.count': options.nodeModules ? Object.keys(options.nodeModules).length.toString() : '0',
138-
'js-execution.engine': options.engine ?? 'auto',
139139
}
140140
}, async () => {
141-
if (options.engine) {
142-
const engine = engineMap.get(options.engine);
143-
if (!engine) {
144-
throw new StackAssertionError(`Unknown JS execution engine: ${options.engine}`);
141+
142+
if (getEnvVariable("STACK_VERCEL_SANDBOX_TOKEN","") != "") {
143+
if (!getNodeEnvironment().includes("prod")) {
144+
throw new StackAssertionError("STACK_VERCEL_SANDBOX_TOKEN is set in non-production environment. We do not use Vercel Sandbox in non-production environments.");
145145
}
146-
return await engine.execute(code, options);
147-
}
148146

149-
const shouldSanityTest = Math.random() < 0.05;
147+
const shouldSanityTest = Math.random() < 0.05;
148+
if (shouldSanityTest) {
149+
runAsynchronouslyAndWaitUntil(runSanityTest(code, options));
150+
}
150151

151-
if (shouldSanityTest) {
152-
runAsynchronouslyAndWaitUntil(runSanityTest(code, options));
152+
return await runWithFallback(code, options);
153+
} else {
154+
return await runWithoutFallback(code, options);
153155
}
154-
155-
// Normal execution: try engines in order with retry for first engine
156-
return await runWithFallback(code, options);
157156
});
158157
}
159158

159+
/**
160+
* Compare two execution results for sanity test equality.
161+
* For error results, we only compare status and message (not stack traces,
162+
* which differ between execution environments).
163+
*/
164+
function areResultsEqual(a: ExecuteResult, b: ExecuteResult): boolean {
165+
if (a.status !== b.status) return false;
166+
167+
if (a.status === 'ok' && b.status === 'ok') {
168+
return JSON.stringify(a.data) === JSON.stringify(b.data);
169+
}
170+
171+
if (a.status === 'error' && b.status === 'error') {
172+
return a.error.message === b.error.message;
173+
}
174+
175+
return false;
176+
}
177+
160178
async function runSanityTest(code: string, options: ExecuteJavascriptOptions) {
161179
const results: Array<{ engine: string, result: unknown }> = [];
162180
const failures: Array<{ engine: string, error: unknown }> = [];
163181

164-
for (const engine of engines) {
182+
for (const [name, engine] of engineMap) {
165183
try {
166184
const result = await engine.execute(code, options);
167-
results.push({ engine: engine.name, result });
185+
results.push({ engine: name, result });
168186
} catch (error) {
169-
failures.push({ engine: engine.name, error });
187+
failures.push({ engine: name, error });
170188
}
171189
}
172190

@@ -181,8 +199,8 @@ async function runSanityTest(code: string, options: ExecuteJavascriptOptions) {
181199
return;
182200
}
183201

184-
const referenceResult = results[0].result;
185-
const allEqual = results.every(r => JSON.stringify(r.result) === JSON.stringify(referenceResult));
202+
const referenceResult = results[0].result as ExecuteResult;
203+
const allEqual = results.every(r => areResultsEqual(r.result as ExecuteResult, referenceResult));
186204
if (!allEqual) {
187205
captureError("js-execution-sanity-test-mismatch", new StackAssertionError(
188206
"JS execution sanity test: engines returned different results",
@@ -191,19 +209,15 @@ async function runSanityTest(code: string, options: ExecuteJavascriptOptions) {
191209
}
192210
}
193211

194-
async function runWithFallback(code: string, options: ExecuteJavascriptOptions): Promise<unknown> {
195-
const errors: Array<{ engine: string, error: unknown }> = [];
212+
async function runWithFallback(code: string, options: ExecuteJavascriptOptions): Promise<ExecuteResult> {
213+
const freestyleEngine = engineMap.get("freestyle")!;
214+
const vercelSandboxEngine = engineMap.get("vercel-sandbox")!;
196215

197-
for (let i = 0; i < engines.length; i++) {
198-
const engine = engines[i];
199-
const isFirstEngine = i === 0;
200-
201-
const maxAttempts = isFirstEngine ? 2 : 1;
202-
203-
const retryResult = await Result.retry(
216+
const maxAttempts = 2;
217+
const retryResult = await Result.retry(
204218
async () => {
205219
try {
206-
const result = await engine.execute(code, options);
220+
const result = await freestyleEngine.execute(code, options);
207221
return Result.ok(result);
208222
} catch (error) {
209223
return Result.error(error);
@@ -213,20 +227,33 @@ async function runWithFallback(code: string, options: ExecuteJavascriptOptions):
213227
{ exponentialDelayBase: 500 }
214228
);
215229

216-
if (retryResult.status === 'ok') {
217-
return retryResult.data;
218-
}
219-
220-
const engineError = retryResult.error;
221-
errors.push({ engine: engine.name, error: engineError });
230+
if (retryResult.status === 'ok') {
231+
return retryResult.data;
232+
}
222233

223-
if (i < engines.length - 1) {
224-
captureError(`js-execution-${engine.name}-failed`, new StackAssertionError(
225-
`JS execution engine '${engine.name}' failed, falling back to next engine`,
226-
{ error: engineError, attempts: retryResult.attempts, innerCode: code, innerOptions: options }
234+
captureError(`js-execution-freestyle-failed`, new StackAssertionError(
235+
`JS execution freestyle engine failed, falling back to vercel sandbox engine`,
236+
{ error: retryResult.error, innerCode: code, innerOptions: options }
237+
));
238+
239+
try {
240+
const result = await vercelSandboxEngine.execute(code, options);
241+
return result;
242+
} catch (error){
243+
captureError(`js-execution-vercel-sandbox-failed`, new StackAssertionError(
244+
`JS execution vercel sandbox engine failed after fallback from freestyle engine`,
245+
{ error: error, innerCode: code, innerOptions: options }
227246
));
228-
}
247+
throw new StackAssertionError("Email rendering service unavailable", { cause: error, innerCode: code, innerOptions: options });
229248
}
249+
}
230250

231-
throw errors[errors.length - 1].error;
251+
async function runWithoutFallback(code: string, options: ExecuteJavascriptOptions): Promise<ExecuteResult> {
252+
const freestyleEngine = engineMap.get("freestyle")!;
253+
try {
254+
const result = await freestyleEngine.execute(code, options);
255+
return result;
256+
} catch (error) {
257+
throw new StackAssertionError("Email rendering service unavailable", { cause: error, innerCode: code, innerOptions: options });
258+
}
232259
}

0 commit comments

Comments
 (0)