Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/eager-lands-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@workflow/nitro": patch
---

Add Vite middleware to handle 404s in workflow routes from Nitro and silence undefined unhandled rejections
5 changes: 5 additions & 0 deletions .changeset/five-planets-push.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@workflow/sveltekit": patch
---

Fix SvelteKit plugin reading deleted files on HMR
9 changes: 9 additions & 0 deletions .claude/commands/demo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
description: Run the 7_full demo workflow
allowed-tools: Bash(curl:*), Bash(npx workflow:*), Bash(pnpm dev)
---


Start the $ARUGMENTS workbench (default to the nextjs turboback workbench available in the workbenches directory). Run it in dev mode, and also start the workflow web UI (run `npx workflow web` inside the appropriate workbench directory).

Then trigger the 7_full.ts workflow example. you can see how to trigger a specific example by looking at the trigger API route for the workbench - it is probably just a POST request using bash (maybe curl) to this endpoint: <http://localhost:3000/api/trigger\?workflowFile\=workflows/7_full.ts\&workflowFn\=handleUserSignup>>
2 changes: 1 addition & 1 deletion .claude/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"Bash(pnpm build:*)",
"Bash(pnpm typecheck:*)"
],
"deny": ["Bash(curl:*)", "Read(./.env)", "Read(./.env.*)"],
"deny": ["Read(./.env)", "Read(./.env.*)"],
"additionalDirectories": ["../workflow-server"]
}
}
7 changes: 3 additions & 4 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ jobs:
project-id: "prj_oTgiz3SGX2fpZuM6E0P38Ts8de6d"
- name: "sveltekit"
project-id: "prj_MqnBLm71ceXGSnm3Fs8i8gBnI23G"
- name: "hono"
project-id: "prj_p0GIEsfl53L7IwVbosPvi9rPSOYW"
env:
TURBO_TOKEN: ${{ secrets.TURBO_TOKEN }}
TURBO_TEAM: ${{ vars.TURBO_TEAM }}
Expand Down Expand Up @@ -181,10 +183,7 @@ jobs:
run: pnpm turbo run build --filter='!./workbench/*'

- name: Resolve symlinks
run: |
if [ -f "workbench/${{ matrix.app.name }}/resolve-symlinks.sh" ]; then
cd workbench/${{ matrix.app.name }} && ./resolve-symlinks.sh
fi
run: ./scripts/resolve-symlinks.sh workbench/${{ matrix.app.name }}

- name: Run E2E Tests
run: cd workbench/${{ matrix.app.name }} && pnpm dev & echo "starting tests in 10 seconds" && sleep 10 && pnpm vitest run packages/core/e2e/dev.test.ts && pnpm run test:e2e
Expand Down
15 changes: 11 additions & 4 deletions packages/core/e2e/dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ export interface DevTestConfig {
apiFileImportPath: string;
/** The workflow file to modify for testing HMR. Defaults to '3_streams.ts' */
testWorkflowFile?: string;
/** The workflows directory relative to appPath. Defaults to 'workflows' */
workflowsDir?: string;
}

function getConfigFromEnv(): DevTestConfig | null {
Expand Down Expand Up @@ -39,6 +41,7 @@ export function createDevTests(config?: DevTestConfig) {
finalConfig.generatedWorkflowPath
);
const testWorkflowFile = finalConfig.testWorkflowFile ?? '3_streams.ts';
const workflowsDir = finalConfig.workflowsDir ?? 'workflows';
const restoreFiles: Array<{ path: string; content: string }> = [];

afterEach(async () => {
Expand All @@ -55,7 +58,7 @@ export function createDevTests(config?: DevTestConfig) {
});

test('should rebuild on workflow change', { timeout: 10_000 }, async () => {
const workflowFile = path.join(appPath, 'workflows', testWorkflowFile);
const workflowFile = path.join(appPath, workflowsDir, testWorkflowFile);

const content = await fs.readFile(workflowFile, 'utf8');

Expand Down Expand Up @@ -83,7 +86,7 @@ export async function myNewWorkflow() {
});

test('should rebuild on step change', { timeout: 10_000 }, async () => {
const stepFile = path.join(appPath, 'workflows', testWorkflowFile);
const stepFile = path.join(appPath, workflowsDir, testWorkflowFile);

const content = await fs.readFile(stepFile, 'utf8');

Expand Down Expand Up @@ -114,7 +117,11 @@ export async function myNewStep() {
'should rebuild on adding workflow file',
{ timeout: 10_000 },
async () => {
const workflowFile = path.join(appPath, 'workflows', 'new-workflow.ts');
const workflowFile = path.join(
appPath,
workflowsDir,
'new-workflow.ts'
);

await fs.writeFile(
workflowFile,
Expand All @@ -132,7 +139,7 @@ export async function myNewStep() {

await fs.writeFile(
apiFile,
`import '${finalConfig.apiFileImportPath}/workflows/new-workflow';
`import '${finalConfig.apiFileImportPath}/${workflowsDir}/new-workflow';
${apiFileContent}`
);

Expand Down
18 changes: 12 additions & 6 deletions packages/core/e2e/e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
])('addTenWorkflow', { timeout: 60_000 }, async (workflow) => {
const run = await triggerWorkflow(workflow, [123]);
const returnValue = await getWorkflowReturnValue(run.runId);
expect(returnValue).toBe(133);

Check failure on line 82 in packages/core/e2e/e2e.test.ts

View workflow job for this annotation

GitHub Actions / E2E Local Dev Tests (nitro - stable)

packages/core/e2e/e2e.test.ts > e2e > addTenWorkflow

AssertionError: expected { error: true, …(4) } to be 133 // Object.is equality - Expected: 133 + Received: { "error": true, "message": "fetch failed", "stack": [ "fetch failed", "at node:internal/deps/undici/undici:14900:13", ], "status": 500, "url": "http://localhost:3000/api/trigger?runId=wrun_01KA1QAZ4TP6FKQ2JEPB91EF52", } ❯ packages/core/e2e/e2e.test.ts:82:25

const { json } = await cliInspectJson(`runs ${run.runId} --withData`);
expect(json).toMatchObject({
Expand All @@ -90,10 +90,11 @@
output: 133,
});
// In local vs. vercel backends, the workflow name is different, so we check for either,
// since this test runs against both.
// since this test runs against both. Also different workbenches have different directory structures.
expect(json.workflowName).toBeOneOf([
`workflow//example/${workflow.workflowFile}//${workflow.workflowFn}`,
`workflow//${workflow.workflowFile}//${workflow.workflowFn}`,
`workflow//src/${workflow.workflowFile}//${workflow.workflowFn}`,
]);
});

Expand Down Expand Up @@ -154,7 +155,10 @@
method: 'POST',
body: JSON.stringify({ token: 'invalid' }),
});
expect(res.status).toBe(404);
// NOTE: For Nitro apps (Vite, Hono, etc.) in dev mode, status 404 does some
// unexpected stuff and could return a Vite SPA fallback or can cause a Hono route to hang.
// This is because Nitro passes the 404 requests to the dev server to handle.
expect(res.status).toBeOneOf([404, 422]);
body = await res.json();
expect(body).toBeNull();

Expand Down Expand Up @@ -578,14 +582,16 @@
expect(returnValue.cause).toHaveProperty('stack');
expect(typeof returnValue.cause.stack).toBe('string');

// Known issue: SvelteKit dev mode has incorrect source map mappings for bundled imports.
// Known issue: vite-based frameworks dev mode has incorrect source map mappings for bundled imports.
// esbuild with bundle:true inlines helpers.ts but source maps incorrectly map to 99_e2e.ts
// This works correctly in production and other frameworks.
// TODO: Investigate esbuild source map generation for bundled modules
const isSvelteKitDevMode =
process.env.APP_NAME === 'sveltekit' && isLocalDeployment();
const isViteBasedFrameworkDevMode =
(process.env.APP_NAME === 'sveltekit' ||
process.env.APP_NAME === 'vite') &&
isLocalDeployment();

if (!isSvelteKitDevMode) {
if (!isViteBasedFrameworkDevMode) {
// Stack trace should include frames from the helper module (helpers.ts)
expect(returnValue.cause.stack).toContain('helpers.ts');
}
Expand Down
1 change: 1 addition & 0 deletions packages/core/e2e/local-build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ describe.each([
'vite',
'sveltekit',
'nuxt',
'hono',
])('e2e', (project) => {
test('builds without errors', { timeout: 180_000 }, async () => {
// skip if we're targeting specific app to test
Expand Down
12 changes: 9 additions & 3 deletions packages/nitro/src/builders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,25 @@ export class LocalBuilder extends BaseBuilder {
inputFiles,
});

const webhookRouteFile = join(this.#outDir, 'webhook.mjs');

await this.createWebhookBundle({
outfile: join(this.#outDir, 'webhook.mjs'),
outfile: webhookRouteFile,
bundle: false,
suppressUndefinedRejections: true,
});
}
}

export function getWorkflowDirs(nitro: Nitro) {
const srcDir = nitro.options.srcDir || nitro.options.rootDir;

return unique(
[
...(nitro.options.workflow?.dirs ?? []),
join(nitro.options.rootDir, 'workflows'),
...nitro.options.scanDirs.map((dir) => join(dir, 'workflows')),
join(srcDir, 'workflows'),
join(srcDir, nitro.options.routesDir || 'routes'),
join(srcDir, nitro.options.apiDir || 'api'),
].map((dir) => resolve(nitro.options.rootDir, dir))
).sort();
}
Expand Down
9 changes: 8 additions & 1 deletion packages/nitro/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,14 @@ function addVirtualHandler(nitro: Nitro, route: string, buildPath: string) {
// Nitro v3+ (native web handlers)
nitro.options.virtual[`#${buildPath}`] = /* js */ `
import { POST } from "${join(nitro.options.buildDir, buildPath)}";
export default ({ req }) => POST(req);
export default async ({ req }) => {
try {
return await POST(req);
} catch (error) {
console.error('Handler error:', error);
return new Response('Internal Server Error', { status: 500 });
}
};
`;
}
}
92 changes: 91 additions & 1 deletion packages/nitro/src/vite.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import type { Nitro } from 'nitro/types';
import type { Plugin } from 'vite';
import type { HotUpdateOptions, Plugin } from 'vite';
import { LocalBuilder } from './builders.js';
import type { ModuleOptions } from './index.js';
import nitroModule from './index.js';
import { workflowRollupPlugin } from './rollup.js';

export function workflow(options?: ModuleOptions): Plugin[] {
let builder: LocalBuilder | undefined;

return [
workflowRollupPlugin(),
{
Expand All @@ -18,9 +21,96 @@ export function workflow(options?: ModuleOptions): Plugin[] {
...options,
_vite: true,
};
if (nitro.options.dev) {
builder = new LocalBuilder(nitro);
}
return nitroModule.setup(nitro);
},
},
// NOTE: This is a workaround because Nitro passes the 404 requests to the dev server to handle.
// For workflow routes, we override to send an empty body to prevent Hono/Vite's SPA fallback.
configureServer(server) {
// Add middleware to intercept 404s on workflow routes before Vite's SPA fallback
return () => {
server.middlewares.use((req, res, next) => {
// Only handle workflow webhook routes
if (!req.url?.startsWith('/.well-known/workflow/v1/')) {
return next();
}

// Wrap writeHead to ensure we send empty body for 404s
const originalWriteHead = res.writeHead;
res.writeHead = function (this: typeof res, ...args: any[]) {
const statusCode = typeof args[0] === 'number' ? args[0] : 200;

// NOTE: Workaround because Nitro passes 404 requests to the vite to handle.
// Causes `webhook route with invalid token` test to fail.
// For 404s on workflow routes, ensure we're sending the right headers
if (statusCode === 404) {
// Set content-length to 0 to prevent Vite from overriding
res.setHeader('Content-Length', '0');
}

// @ts-expect-error - Complex overload signature
return originalWriteHead.apply(this, args);
} as any;

next();
});
};
},
// TODO: Move this to @workflow/vite or something since this is vite specific
async hotUpdate(options: HotUpdateOptions) {
const { file, server, read } = options;

// Check if this is a TS/JS file that might contain workflow directives
const jsTsRegex = /\.(ts|tsx|js|jsx|mjs|cjs)$/;
if (!jsTsRegex.test(file)) {
return;
}

// Read the file to check for workflow/step directives
let content: string;
try {
content = await read();
} catch {
// File might have been deleted - trigger rebuild to update generated routes
console.log('Workflow file deleted, rebuilding...');
if (builder) {
await builder.build();
}
// NOTE: Might be too aggressive
server.ws.send({
type: 'full-reload',
path: '*',
});
return;
}

const useWorkflowPattern = /^\s*(['"])use workflow\1;?\s*$/m;
const useStepPattern = /^\s*(['"])use step\1;?\s*$/m;

if (
!useWorkflowPattern.test(content) &&
!useStepPattern.test(content)
) {
return;
}

// Trigger full reload - this will cause Nitro's dev:reload hook to fire,
// which will rebuild workflows and update routes
console.log('Workflow file changed, rebuilding...');
if (builder) {
await builder.build();
}
Comment on lines +103 to +105
Copy link
Contributor

Choose a reason for hiding this comment

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

The hotUpdate hook in the Nitro Vite plugin calls builder.build() without error handling when a workflow file changes. This can cause unhandled promise rejections that crash the dev server.

View Details
📝 Patch Details
diff --git a/packages/nitro/src/vite.ts b/packages/nitro/src/vite.ts
index f210250..971b40e 100644
--- a/packages/nitro/src/vite.ts
+++ b/packages/nitro/src/vite.ts
@@ -77,7 +77,13 @@ export function workflow(options?: ModuleOptions): Plugin[] {
           // File might have been deleted - trigger rebuild to update generated routes
           console.log('Workflow file deleted, rebuilding...');
           if (builder) {
-            await builder.build();
+            try {
+              await builder.build();
+            } catch (buildError) {
+              // Build might fail if files are being deleted during test cleanup
+              // Log but don't crash - the next successful change will trigger a rebuild
+              console.error('Build failed during file deletion:', buildError);
+            }
           }
           // NOTE: Might be too aggressive
           server.ws.send({
@@ -101,7 +107,14 @@ export function workflow(options?: ModuleOptions): Plugin[] {
         // which will rebuild workflows and update routes
         console.log('Workflow file changed, rebuilding...');
         if (builder) {
-          await builder.build();
+          try {
+            await builder.build();
+          } catch (buildError) {
+            // Build might fail if files are being modified/deleted during test cleanup
+            // Log but don't crash - the next successful change will trigger a rebuild
+            console.error('Build failed during HMR:', buildError);
+            return;
+          }
         }
         server.ws.send({
           type: 'full-reload',

Analysis

Unhandled builder.build() errors in Nitro Vite plugin hotUpdate hook

What fails: The Nitro Vite plugin's hotUpdate handler at lines 80 and 104 calls await builder.build() without error handling. When builder.build() throws an error (e.g., from esbuild compilation failures during file modifications), this causes an unhandled promise rejection that can crash or destabilize the Vite dev server.

How to reproduce:

  1. Start Nitro dev server with the workflow plugin enabled
  2. Create a syntax error in a workflow file (e.g., a file containing 'use workflow')
  3. The hotUpdate hook triggers builder.build()
  4. Builder fails due to syntax error in esbuild compilation
  5. The unhandled promise rejection crashes the dev server or logs an unhandled rejection warning

Result: Dev server crashes or becomes unstable with unhandled promise rejection. Server logs show rejection from builder.build() without proper error handling.

Expected: Errors from builder.build() should be caught, logged, and gracefully handled without crashing the dev server. A page reload should still be triggered to allow developers to fix the error and recover.

Evidence: The SvelteKit plugin in the same codebase (packages/sveltekit/src/plugin.ts, lines 141-148 and 149-159) implements identical logic with proper try-catch error handling around both builder.build() calls. These were added in commit b2c6e3b ("fix: normalize workbench tests #292") along with comments noting "Build might fail if files are being deleted during test cleanup". The Nitro plugin was not updated in that commit despite having the same vulnerability pattern. Per Vite issue #20835 and PR #18713, unhandled errors in plugin hooks can cause issues - Vite itself fixed similar unhandled rejection bugs in its full reload handler.

server.ws.send({
type: 'full-reload',
path: '*',
});

// Let Vite handle the normal HMR for the changed file
return;
},
},
];
}
5 changes: 3 additions & 2 deletions packages/nitro/test/dirs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const nitroMock = (dirs: string[]) => {
describe('nitro:getWorkflowDirs', () => {
test('default dirs', () => {
const result = getWorkflowDirs(nitroMock([]));
expect(result).toEqual(['/root/server/workflows', '/root/workflows']);
expect(result).toEqual(['/root/api', '/root/routes', '/root/workflows']);
});

test('custom dirs', () => {
Expand All @@ -24,8 +24,9 @@ describe('nitro:getWorkflowDirs', () => {
);
expect(result).toEqual([
'/custom/dir2',
'/root/api',
'/root/relative/dir1',
'/root/server/workflows',
'/root/routes',
'/root/workflows',
]);
});
Expand Down
6 changes: 4 additions & 2 deletions packages/sveltekit/src/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ async function convertSvelteKitRequest(request) {

export class SvelteKitBuilder extends BaseBuilder {
constructor(config?: Partial<SvelteKitConfig>) {
const workingDir = config?.workingDir || process.cwd();

super({
...config,
dirs: ['workflows'],
dirs: ['workflows', 'src/workflows', 'routes', 'src/routes'],
buildTarget: 'sveltekit' as const,
stepsBundlePath: '', // unused in base
workflowsBundlePath: '', // unused in base
webhookBundlePath: '', // unused in base
workingDir: config?.workingDir || process.cwd(),
workingDir,
});
}

Expand Down
26 changes: 24 additions & 2 deletions packages/sveltekit/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,22 @@ export function workflowPlugin(): Plugin {
}

// Read the file to check for workflow/step directives
const content = await read();
let content: string;
try {
content = await read();
} catch {
// File might have been deleted - trigger rebuild to update generated routes
console.log('Workflow file deleted, regenerating routes...');
try {
await builder.build();
} catch (buildError) {
// Build might fail if files are being deleted during test cleanup
// Log but don't crash - the next successful change will trigger a rebuild
console.error('Build failed during file deletion:', buildError);
}
return;
}

const useWorkflowPattern = /^\s*(['"])use workflow\1;?\s*$/m;
const useStepPattern = /^\s*(['"])use step\1;?\s*$/m;

Expand All @@ -113,7 +128,14 @@ export function workflowPlugin(): Plugin {

// Rebuild everything - simpler and more reliable than tracking individual files
console.log('Workflow file changed, regenerating routes...');
await builder.build();
try {
await builder.build();
} catch (buildError) {
// Build might fail if files are being modified/deleted during test cleanup
// Log but don't crash - the next successful change will trigger a rebuild
console.error('Build failed during HMR:', buildError);
return;
}

// Trigger full reload of workflow routes
server.ws.send({
Expand Down
Loading
Loading