Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"chokidar-cli": "^3.0.0",
"dotenv": "^16.4.5",
"dotenv-cli": "^7.3.0",
"freestyle-sandboxes": "^0.0.92",
"freestyle-sandboxes": "^0.1.5",
"jose": "^5.2.2",
"json-diff": "^1.0.6",
"next": "16.1.1",
Expand Down
69 changes: 31 additions & 38 deletions apps/backend/src/lib/email-rendering.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Freestyle } from '@/lib/freestyle';
import { emptyEmailTheme } from '@stackframe/stack-shared/dist/helpers/emails';
import { getEnvVariable } from '@stackframe/stack-shared/dist/utils/env';
import { captureError, StackAssertionError } from '@stackframe/stack-shared/dist/utils/errors';
import { StackAssertionError } from '@stackframe/stack-shared/dist/utils/errors';
import { bundleJavaScript } from '@stackframe/stack-shared/dist/utils/esbuild';
import { get, has } from '@stackframe/stack-shared/dist/utils/objects';
import { Result } from "@stackframe/stack-shared/dist/utils/results";
Expand Down Expand Up @@ -117,7 +117,14 @@ export async function renderEmailWithTemplate(
`,
"/entry.js": deindent`
import { renderAll } from "./render.tsx";
export default renderAll;
export default async () => {
try {
const result = await renderAll();
return { _status: "ok", _data: result };
} catch (e) {
return { _status: "error", _error: String(e) };
}
};
`,
}, {
keepAsImports: ['arktype', 'react', 'react/jsx-runtime', '@react-email/components'],
Expand All @@ -136,21 +143,11 @@ export async function renderEmailWithTemplate(
"@react-email/components": "0.1.1",
"arktype": "2.1.20",
};
const executeResult = await freestyle.executeScript(result.data, { nodeModules });
const executeResult = await freestyle.executeScript(result.data, { config: { nodeModules } });
if (executeResult.status === "error") {
return Result.error(`${executeResult.error}`);
}
if (!executeResult.data.result) {
const noResultError = new StackAssertionError("No result from Freestyle", {
executeResult,
templateOrDraftComponent,
themeComponent,
options,
});
captureError("freestyle-no-result", noResultError);
throw noResultError;
}
return Result.ok(executeResult.data.result as { html: string, text: string, subject: string, notificationCategory: string });
return Result.ok(executeResult.data as { html: string, text: string, subject: string, notificationCategory: string });
Comment on lines 147 to +150
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add defensive validation before type casting.

The code checks for error status but doesn't validate that executeResult.data actually contains the expected { html, text, subject, notificationCategory } structure before casting. According to coding guidelines, prefer defensive coding with explicit error messages.

🛡️ Proposed defensive validation
   if (executeResult.status === "error") {
     return Result.error(`${executeResult.error}`);
   }
-  return Result.ok(executeResult.data as { html: string, text: string, subject: string, notificationCategory: string });
+  const data = executeResult.data as any;
+  if (!data || typeof data.html !== 'string' || typeof data.text !== 'string') {
+    throw new StackAssertionError("Freestyle returned invalid email rendering result", { data });
+  }
+  return Result.ok(data as { html: string, text: string, subject?: string, notificationCategory?: string });

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In @apps/backend/src/lib/email-rendering.tsx around lines 147 - 150, The code
returns Result.ok by blindly casting executeResult.data to the expected shape;
add defensive runtime validation of executeResult.data before casting: check
that executeResult.data exists and that html, text, subject, and
notificationCategory are present and of the expected types (strings), and if any
check fails return Result.error with a clear message indicating which field is
missing/invalid rather than casting; update the branch that currently does
Result.ok(executeResult.data as { html: string, text: string, subject: string,
notificationCategory: string }) to perform these checks and only call Result.ok
with the validated object.

}

// unused, but kept for reference & in case we need it again
Expand Down Expand Up @@ -221,7 +218,14 @@ export async function renderEmailsWithTemplateBatched(
`,
"/entry.js": deindent`
import { renderAll } from "./render.tsx";
export default renderAll;
export default async () => {
try {
const result = await renderAll();
return { _status: "ok", _data: result };
} catch (e) {
return { _status: "error", _error: String(e) };
}
};
`,
}, {
keepAsImports: ['arktype', 'react', 'react/jsx-runtime', '@react-email/components'],
Expand All @@ -240,21 +244,11 @@ export async function renderEmailsWithTemplateBatched(
"@react-email/components": "0.1.1",
"arktype": "2.1.20",
};
const executeResult = await freestyle.executeScript(result.data, { nodeModules });
const executeResult = await freestyle.executeScript(result.data, { config: { nodeModules } });
if (executeResult.status === "error") {
return Result.error(executeResult.error);
}
if (!executeResult.data.result) {
const noResultError = new StackAssertionError("No result from Freestyle", {
executeResult,
templateOrDraftComponent,
themeComponent,
inputs,
});
captureError("freestyle-no-result", noResultError);
throw noResultError;
}
return Result.ok(executeResult.data.result as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
return Result.ok(executeResult.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
Comment on lines 248 to +251
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add defensive validation before type casting.

Similar to line 150, this casts the result without validating the data structure. Add defensive checks to ensure executeResult.data is an array with the expected shape.

🛡️ Proposed defensive validation
   if (executeResult.status === "error") {
     return Result.error(executeResult.error);
   }
-  return Result.ok(executeResult.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
+  const data = executeResult.data as any;
+  if (!Array.isArray(data)) {
+    throw new StackAssertionError("Freestyle returned non-array result for batched rendering", { data });
+  }
+  return Result.ok(data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (executeResult.status === "error") {
return Result.error(executeResult.error);
}
if (!executeResult.data.result) {
const noResultError = new StackAssertionError("No result from Freestyle", {
executeResult,
templateOrDraftComponent,
themeComponent,
inputs,
});
captureError("freestyle-no-result", noResultError);
throw noResultError;
}
return Result.ok(executeResult.data.result as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
return Result.ok(executeResult.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
if (executeResult.status === "error") {
return Result.error(executeResult.error);
}
// Cast to any for runtime validation; the type system cannot verify executeResult.data structure at compile time
const data = executeResult.data as any;
if (!Array.isArray(data)) {
throw new StackAssertionError("Freestyle returned non-array result for batched rendering", { data });
}
return Result.ok(data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
🤖 Prompt for AI Agents
In @apps/backend/src/lib/email-rendering.tsx around lines 248 - 251, The code
currently casts executeResult.data to the expected array shape without
validation; add defensive checks (e.g., Array.isArray(executeResult.data) and
that every item has string properties html and text and optional string
subject/notificationCategory) before calling Result.ok in place of the direct
cast used around executeResult and Result.ok; if validation fails return
Result.error with a helpful message. Implement this as a small
type-guard/validator function (e.g., isRenderedEmailArray or
validateRenderedEmails) and use it where executeResult is handled so you never
assume the shape when calling Result.ok.

}

export type RenderEmailRequestForTenancy = {
Expand Down Expand Up @@ -340,7 +334,14 @@ export async function renderEmailsForTenancyBatched(requests: RenderEmailRequest

files["/entry.js"] = deindent`
import { renderAll } from "./render.tsx";
export default renderAll;
export default async function() {
try {
const result = await renderAll();
return { _status: "ok", _data: result };
} catch (e) {
return { _status: "error", _error: String(e) };
}
};
`;

const bundle = await bundleJavaScript(files as Record<string, string> & { '/entry.js': string }, {
Expand All @@ -362,20 +363,12 @@ export async function renderEmailsForTenancyBatched(requests: RenderEmailRequest
"arktype": "2.1.20",
};

const execution = await freestyle.executeScript(bundle.data, { nodeModules });
const execution = await freestyle.executeScript(bundle.data, { config: { nodeModules } });
if (execution.status === "error") {
return Result.error(execution.error);
}
if (!execution.data.result) {
const noResultError = new StackAssertionError("No result from Freestyle", {
execution,
requests,
});
captureError("freestyle-no-result", noResultError);
throw noResultError;
}

return Result.ok(execution.data.result as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
return Result.ok(execution.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
Comment on lines 367 to +371
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add defensive validation before type casting.

Same as line 251, add defensive checks to ensure the result is a valid array before casting.

🛡️ Proposed defensive validation
   if (execution.status === "error") {
     return Result.error(execution.error);
   }
 
-  return Result.ok(execution.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
+  const data = execution.data as any;
+  if (!Array.isArray(data)) {
+    throw new StackAssertionError("Freestyle returned non-array result for tenancy batched rendering", { data });
+  }
+  return Result.ok(data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (execution.status === "error") {
return Result.error(execution.error);
}
if (!execution.data.result) {
const noResultError = new StackAssertionError("No result from Freestyle", {
execution,
requests,
});
captureError("freestyle-no-result", noResultError);
throw noResultError;
}
return Result.ok(execution.data.result as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
return Result.ok(execution.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
if (execution.status === "error") {
return Result.error(execution.error);
}
const data = execution.data as any;
if (!Array.isArray(data)) {
throw new StackAssertionError("Freestyle returned non-array result for tenancy batched rendering", { data });
}
return Result.ok(data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
🤖 Prompt for AI Agents
In @apps/backend/src/lib/email-rendering.tsx around lines 367 - 371, The return
is casting execution.data to an array without validation; add defensive checks
similar to the earlier validation at line 251: verify execution.data is an Array
and every item is an object with required string properties (html and text,
optional subject and notificationCategory) before returning Result.ok; if the
check fails return Result.error with a clear message (e.g., "Invalid email
rendering data") so callers don’t receive an unsafe cast.

}

const findComponentValueUtil = `import React from 'react';
Expand Down
45 changes: 23 additions & 22 deletions apps/backend/src/lib/freestyle.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { traceSpan } from '@/utils/telemetry';
import { getEnvVariable, getNodeEnvironment } from '@stackframe/stack-shared/dist/utils/env';
import { StackAssertionError } from '@stackframe/stack-shared/dist/utils/errors';
import { parseJson } from '@stackframe/stack-shared/dist/utils/json';
import { Result } from '@stackframe/stack-shared/dist/utils/results';
import { FreestyleSandboxes } from 'freestyle-sandboxes';
import { Freestyle as FreestyleClient } from 'freestyle-sandboxes';

// Extract options type from the SDK's serverless.runs.create method, excluding 'code'
// Make 'config' optional since we provide a default empty config
type ServerlessRunsCreateParams = Parameters<FreestyleClient['serverless']['runs']['create']>[0];
export type ExecuteScriptOptions = Partial<Omit<ServerlessRunsCreateParams, 'code'>>;

export class Freestyle {
private freestyle: FreestyleSandboxes;
private freestyle: FreestyleClient;

constructor(options: { apiKey?: string } = {}) {
const apiKey = options.apiKey || getEnvVariable("STACK_FREESTYLE_API_KEY");
Expand All @@ -18,39 +22,36 @@ export class Freestyle {
const prefix = getEnvVariable("NEXT_PUBLIC_STACK_PORT_PREFIX", "81");
baseUrl = `http://localhost:${prefix}22`;
}
this.freestyle = new FreestyleSandboxes({
this.freestyle = new FreestyleClient({
apiKey,
baseUrl,
});
}

async executeScript(script: string, options?: Parameters<FreestyleSandboxes['executeScript']>[1]) {
async executeScript(script: string, options?: ExecuteScriptOptions) {
return await traceSpan({
description: 'freestyle.executeScript',
attributes: {
'freestyle.operation': 'executeScript',
'freestyle.script.length': script.length.toString(),
'freestyle.nodeModules.count': options?.nodeModules ? Object.keys(options.nodeModules).length.toString() : '0',
'freestyle.nodeModules.count': options?.config?.nodeModules ? Object.keys(options.config.nodeModules).length.toString() : '0',
}
}, async () => {
try {
return Result.ok(Result.orThrow(await Result.retry(async () => {
try {
return Result.ok(await this.freestyle.executeScript(script, options));
} catch (e: unknown) {
if (e instanceof Error && (e as any).code === "ETIMEDOUT") {
return Result.error(new StackAssertionError("Freestyle timeout", { cause: e }));
}
throw e;
}
}, 3)));
const response = await this.freestyle.serverless.runs.create({
...options,
code: script,
config: options?.config ?? {},
});

const result = response.result as { _status: string, _data: unknown, _error: string };
if (result._status === 'error') {
return Result.error(result._error);
}
return Result.ok(result._data);
} catch (e: unknown) {
// for whatever reason, Freestyle's errors are sometimes returned in JSON.parse(e.error.error).error (lol)
const wrap1 = e && typeof e === "object" && "error" in e ? e.error : e;
const wrap2 = wrap1 && typeof wrap1 === "object" && "error" in wrap1 ? wrap1.error : wrap1;
const wrap3 = wrap2 && typeof wrap2 === "string" ? Result.or(parseJson(wrap2), wrap2) : wrap2;
const wrap4 = wrap3 && typeof wrap3 === "object" && "error" in wrap3 ? wrap3.error : wrap3;
return Result.error(`${wrap4}`);
const message = e instanceof Error ? e.message : String(e);
return Result.error(message);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ describe("with valid credentials", () => {
}
`);

await wait(5_000);
await wait(10_000);

const response = await niceBackendFetch("/api/v1/internal/failed-emails-digest", {
method: "POST",
Expand Down Expand Up @@ -158,7 +158,7 @@ describe("with valid credentials", () => {
const { projectOwnerMailbox } = await testFailedEmails(true);

const messages = await projectOwnerMailbox.fetchMessages();
expect(messages.filter(msg => !msg.subject.includes("Sign in to"))).toMatchInlineSnapshot(`[]`);
expect(messages.filter(msg => !msg.subject.includes("Sign in"))).toMatchInlineSnapshot(`[]`);
});

// TODO: failed emails digest is currently disabled, fix that and then re-enable this test
Expand Down
Loading