- 
                Notifications
    
You must be signed in to change notification settings  - Fork 72
 
refactor: improve error handling in bundle creation #82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
          🦋 Changeset detectedLatest commit: 4698a8f The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
 Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR  | 
    
| 
           The latest updates on your projects. Learn more about Vercel for GitHub. 
  | 
    
ed696c2    to
    9edd6a2      
    Compare
  
    93750d8    to
    bfd4924      
    Compare
  
    9edd6a2    to
    96f5dc8      
    Compare
  
    bfd4924    to
    7254323      
    Compare
  
    4294532    to
    cbd8775      
    Compare
  
    ffdfe52    to
    7f7e745      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Suggestions:
- Resource leak: When esbuild errors occur, the 
logEsbuildMessagescall throws an exception before the esbuild context is disposed, causing resource leaks. 
View Details
📝 Patch Details
diff --git a/packages/builders/src/base-builder.ts b/packages/builders/src/base-builder.ts
index fb94583..1f8662d 100644
--- a/packages/builders/src/base-builder.ts
+++ b/packages/builders/src/base-builder.ts
@@ -320,28 +320,33 @@ export abstract class BaseBuilder {
       external: this.config.externalPackages || [],
     });
 
-    const stepsResult = await esbuildCtx.rebuild();
+    try {
+      const stepsResult = await esbuildCtx.rebuild();
 
-    this.logEsbuildMessages(stepsResult, 'steps bundle creation');
-    console.log('Created steps bundle', `${Date.now() - stepsBundleStart}ms`);
+      this.logEsbuildMessages(stepsResult, 'steps bundle creation');
+      console.log('Created steps bundle', `${Date.now() - stepsBundleStart}ms`);
 
-    const partialWorkflowManifest = {
-      steps: workflowManifest.steps,
-    };
-    // always write to debug file
-    await this.writeDebugFile(
-      join(dirname(outfile), 'manifest'),
-      partialWorkflowManifest,
-      true
-    );
+      const partialWorkflowManifest = {
+        steps: workflowManifest.steps,
+      };
+      // always write to debug file
+      await this.writeDebugFile(
+        join(dirname(outfile), 'manifest'),
+        partialWorkflowManifest,
+        true
+      );
 
-    // Create .gitignore in .swc directory
-    await this.createSwcGitignore();
+      // Create .gitignore in .swc directory
+      await this.createSwcGitignore();
 
-    if (this.config.watch) {
-      return esbuildCtx;
+      if (this.config.watch) {
+        return esbuildCtx;
+      }
+    } finally {
+      if (!this.config.watch) {
+        await esbuildCtx.dispose();
+      }
     }
-    await esbuildCtx.dispose();
   }
 
   protected async createWorkflowsBundle({
@@ -419,120 +424,125 @@ export abstract class BaseBuilder {
         createNodeModuleErrorPlugin(),
       ],
     });
-    const interimBundle = await interimBundleCtx.rebuild();
-
-    this.logEsbuildMessages(interimBundle, 'intermediate workflow bundle');
-    console.log(
-      'Created intermediate workflow bundle',
-      `${Date.now() - bundleStartTime}ms`
-    );
-    const partialWorkflowManifest = {
-      workflows: workflowManifest.workflows,
-    };
-    await this.writeDebugFile(
-      join(dirname(outfile), 'manifest'),
-      partialWorkflowManifest,
-      true
-    );
+    try {
+      const interimBundle = await interimBundleCtx.rebuild();
 
-    if (this.config.workflowManifestPath) {
-      const resolvedPath = resolve(
-        process.cwd(),
-        this.config.workflowManifestPath
+      this.logEsbuildMessages(interimBundle, 'intermediate workflow bundle');
+      console.log(
+        'Created intermediate workflow bundle',
+        `${Date.now() - bundleStartTime}ms`
       );
-      let prefix = '';
-
-      if (resolvedPath.endsWith('.cjs')) {
-        prefix = 'module.exports = ';
-      } else if (
-        resolvedPath.endsWith('.js') ||
-        resolvedPath.endsWith('.mjs')
-      ) {
-        prefix = 'export default ';
-      }
-
-      await mkdir(dirname(resolvedPath), { recursive: true });
-      await writeFile(
-        resolvedPath,
-        prefix + JSON.stringify(workflowManifest.workflows, null, 2)
+      const partialWorkflowManifest = {
+        workflows: workflowManifest.workflows,
+      };
+      await this.writeDebugFile(
+        join(dirname(outfile), 'manifest'),
+        partialWorkflowManifest,
+        true
       );
-    }
 
-    // Create .gitignore in .swc directory
-    await this.createSwcGitignore();
+      if (this.config.workflowManifestPath) {
+        const resolvedPath = resolve(
+          process.cwd(),
+          this.config.workflowManifestPath
+        );
+        let prefix = '';
+
+        if (resolvedPath.endsWith('.cjs')) {
+          prefix = 'module.exports = ';
+        } else if (
+          resolvedPath.endsWith('.js') ||
+          resolvedPath.endsWith('.mjs')
+        ) {
+          prefix = 'export default ';
+        }
 
-    if (!interimBundle.outputFiles || interimBundle.outputFiles.length === 0) {
-      throw new Error('No output files generated from esbuild');
-    }
+        await mkdir(dirname(resolvedPath), { recursive: true });
+        await writeFile(
+          resolvedPath,
+          prefix + JSON.stringify(workflowManifest.workflows, null, 2)
+        );
+      }
 
-    const bundleFinal = async (interimBundle: string) => {
-      const workflowBundleCode = interimBundle;
+      // Create .gitignore in .swc directory
+      await this.createSwcGitignore();
 
-      // Create the workflow function handler with proper linter suppressions
-      const workflowFunctionCode = `// biome-ignore-all lint: generated file
-/* eslint-disable */
-import { workflowEntrypoint } from 'workflow/runtime';
+      if (!interimBundle.outputFiles || interimBundle.outputFiles.length === 0) {
+        throw new Error('No output files generated from esbuild');
+      }
 
-const workflowCode = \`${workflowBundleCode.replace(/[\\`$]/g, '\\<!-- PATCH_DETAILS_PLACEHOLDER -->')}\`;
+      const bundleFinal = async (interimBundle: string) => {
+        const workflowBundleCode = interimBundle;
 
-export const POST = workflowEntrypoint(workflowCode);`;
+        // Create the workflow function handler with proper linter suppressions
+        const workflowFunctionCode = `// biome-ignore-all lint: generated file
+  /* eslint-disable */
+  import { workflowEntrypoint } from 'workflow/runtime';
 
-      // we skip the final bundling step for Next.js so it can bundle itself
-      if (!bundleFinalOutput) {
-        if (!outfile) {
-          throw new Error(`Invariant: missing outfile for workflow bundle`);
-        }
-        // Ensure the output directory exists
-        const outputDir = dirname(outfile);
-        await mkdir(outputDir, { recursive: true });
+  const workflowCode = \`${workflowBundleCode.replace(/[\\`$]/g, '\\<!-- PATCH_DETAILS_PLACEHOLDER -->')}\`;
 
-        await writeFile(outfile, workflowFunctionCode);
-        return;
-      }
+  export const POST = workflowEntrypoint(workflowCode);`;
 
-      const bundleStartTime = Date.now();
-
-      // Now bundle this so we can resolve the @workflow/core dependency
-      // we could remove this if we do nft tracing or similar instead
-      const finalWorkflowResult = await esbuild.build({
-        banner: {
-          js: '// biome-ignore-all lint: generated file\n/* eslint-disable */\n',
-        },
-        stdin: {
-          contents: workflowFunctionCode,
-          resolveDir: this.config.workingDir,
-          sourcefile: 'virtual-entry.js',
-          loader: 'js',
-        },
-        outfile,
-        // TODO: investigate proper source map support
-        sourcemap: EMIT_SOURCEMAPS_FOR_DEBUGGING,
-        absWorkingDir: this.config.workingDir,
-        bundle: true,
-        format,
-        platform: 'node',
-        target: 'es2022',
-        write: true,
-        keepNames: true,
-        minify: false,
-        external: ['@aws-sdk/credential-provider-web-identity'],
-      });
+        // we skip the final bundling step for Next.js so it can bundle itself
+        if (!bundleFinalOutput) {
+          if (!outfile) {
+            throw new Error(`Invariant: missing outfile for workflow bundle`);
+          }
+          // Ensure the output directory exists
+          const outputDir = dirname(outfile);
+          await mkdir(outputDir, { recursive: true });
 
-      this.logEsbuildMessages(finalWorkflowResult, 'final workflow bundle');
-      console.log(
-        'Created final workflow bundle',
-        `${Date.now() - bundleStartTime}ms`
-      );
-    };
-    await bundleFinal(interimBundle.outputFiles[0].text);
+          await writeFile(outfile, workflowFunctionCode);
+          return;
+        }
+
+        const bundleStartTime = Date.now();
 
-    if (this.config.watch) {
-      return {
-        interimBundleCtx,
-        bundleFinal,
+        // Now bundle this so we can resolve the @workflow/core dependency
+        // we could remove this if we do nft tracing or similar instead
+        const finalWorkflowResult = await esbuild.build({
+          banner: {
+            js: '// biome-ignore-all lint: generated file\n/* eslint-disable */\n',
+          },
+          stdin: {
+            contents: workflowFunctionCode,
+            resolveDir: this.config.workingDir,
+            sourcefile: 'virtual-entry.js',
+            loader: 'js',
+          },
+          outfile,
+          // TODO: investigate proper source map support
+          sourcemap: EMIT_SOURCEMAPS_FOR_DEBUGGING,
+          absWorkingDir: this.config.workingDir,
+          bundle: true,
+          format,
+          platform: 'node',
+          target: 'es2022',
+          write: true,
+          keepNames: true,
+          minify: false,
+          external: ['@aws-sdk/credential-provider-web-identity'],
+        });
+
+        this.logEsbuildMessages(finalWorkflowResult, 'final workflow bundle');
+        console.log(
+          'Created final workflow bundle',
+          `${Date.now() - bundleStartTime}ms`
+        );
       };
+      await bundleFinal(interimBundle.outputFiles[0].text);
+
+      if (this.config.watch) {
+        return {
+          interimBundleCtx,
+          bundleFinal,
+        };
+      }
+    } finally {
+      if (!this.config.watch) {
+        await interimBundleCtx.dispose();
+      }
     }
-    await interimBundleCtx.dispose();
   }
 
   protected async createClientLibrary(): Promise<void> {
Analysis
Resource leak in esbuild context disposal when build errors occur
What fails: In createStepsBundle() and createWorkflowsBundle() methods, esbuild contexts are not disposed when logEsbuildMessages() throws exceptions due to build errors, causing resource leaks.
How to reproduce:
# Trigger a build with esbuild errors in steps/workflows
# The logEsbuildMessages() method throws by default when errors are present
# Context is not disposed because the exception prevents reaching the dispose() callResult: When esbuild errors occur during builds:
createStepsBundle(): Context created at line 279,logEsbuildMessages()throws at line 325 (default throwOnError=true), but dispose() at line 344 is never reachedcreateWorkflowsBundle(): Context created at line 389,logEsbuildMessages()throws at line 424, but dispose() at line 535 is never reached
According to esbuild documentation, dispose() must be called to "free up resources" held by a BuildContext. Without it, system resources remain allocated.
Expected: Resources should be cleaned up even when build errors cause exceptions. The fix wraps context usage in try-finally blocks to ensure dispose() is called regardless of whether an error is thrown.
View Details
📝 Patch Details
diff --git a/packages/builders/src/base-builder.ts b/packages/builders/src/base-builder.ts
index fb94583..f666a04 100644
--- a/packages/builders/src/base-builder.ts
+++ b/packages/builders/src/base-builder.ts
@@ -419,23 +419,25 @@ export abstract class BaseBuilder {
         createNodeModuleErrorPlugin(),
       ],
     });
-    const interimBundle = await interimBundleCtx.rebuild();
 
-    this.logEsbuildMessages(interimBundle, 'intermediate workflow bundle');
-    console.log(
+    try {
+      const interimBundle = await interimBundleCtx.rebuild();
+
+      this.logEsbuildMessages(interimBundle, 'intermediate workflow bundle');
+      console.log(
       'Created intermediate workflow bundle',
       `${Date.now() - bundleStartTime}ms`
-    );
-    const partialWorkflowManifest = {
+      );
+      const partialWorkflowManifest = {
       workflows: workflowManifest.workflows,
-    };
-    await this.writeDebugFile(
+      };
+      await this.writeDebugFile(
       join(dirname(outfile), 'manifest'),
       partialWorkflowManifest,
       true
-    );
+      );
 
-    if (this.config.workflowManifestPath) {
+      if (this.config.workflowManifestPath) {
       const resolvedPath = resolve(
         process.cwd(),
         this.config.workflowManifestPath
@@ -456,16 +458,16 @@ export abstract class BaseBuilder {
         resolvedPath,
         prefix + JSON.stringify(workflowManifest.workflows, null, 2)
       );
-    }
+      }
 
-    // Create .gitignore in .swc directory
-    await this.createSwcGitignore();
+      // Create .gitignore in .swc directory
+      await this.createSwcGitignore();
 
-    if (!interimBundle.outputFiles || interimBundle.outputFiles.length === 0) {
+      if (!interimBundle.outputFiles || interimBundle.outputFiles.length === 0) {
       throw new Error('No output files generated from esbuild');
-    }
+      }
 
-    const bundleFinal = async (interimBundle: string) => {
+      const bundleFinal = async (interimBundle: string) => {
       const workflowBundleCode = interimBundle;
 
       // Create the workflow function handler with proper linter suppressions
@@ -523,16 +525,20 @@ export const POST = workflowEntrypoint(workflowCode);`;
         'Created final workflow bundle',
         `${Date.now() - bundleStartTime}ms`
       );
-    };
-    await bundleFinal(interimBundle.outputFiles[0].text);
-
-    if (this.config.watch) {
-      return {
-        interimBundleCtx,
-        bundleFinal,
       };
+      await bundleFinal(interimBundle.outputFiles[0].text);
+
+      if (this.config.watch) {
+        return {
+          interimBundleCtx,
+          bundleFinal,
+        };
+      }
+    } finally {
+      if (!this.config.watch) {
+        await interimBundleCtx.dispose();
+      }
     }
-    await interimBundleCtx.dispose();
   }
 
   protected async createClientLibrary(): Promise<void> {
Analysis
Resource leak in createWorkflowsBundle() when logEsbuildMessages throws
What fails: The esbuild context created at line 389 in createWorkflowsBundle() may not be disposed if logEsbuildMessages() throws an error at line 424. The logEsbuildMessages() method throws when esbuild errors are present and throwOnError=true (default behavior), skipping the disposal code that would normally run at line 535.
How to reproduce: Trigger an esbuild error during intermediate workflow bundle creation when not in watch mode:
# Create a workflow file with a syntax error that esbuild will catch
# Run the builder with bundleFinalOutput=true and watch=false
# Observe: The esbuild context is not disposed despite the errorResult: The esbuild context resource is leaked because the execution path from line 424 (error throw in logEsbuildMessages) jumps past line 535 (await interimBundleCtx.dispose()), preventing proper cleanup.
Expected behavior: According to esbuild documentation, calling dispose() on a context object is required to "free up resources" held by the context. This is particularly critical in non-watch mode where the context is not expected to be retained.
Fix: Wrapped the entire intermediate bundle processing logic (lines 422-534) in a try-finally block. The finally block checks !this.config.watch and calls await interimBundleCtx.dispose() regardless of whether an error occurred during logEsbuildMessages() or other operations within the try block.
cbd8775    to
    0364bf5      
    Compare
  
    Enhances logEsbuildMessages to throw on critical errors by default, ensuring build failures are properly surfaced instead of being silently logged. Changes: - Added throwOnError parameter to logEsbuildMessages (defaults to true) - When errors occur, now throws with formatted error messages - Collects all error messages and locations for comprehensive error reporting This prevents builds from appearing successful when esbuild encounters critical errors, improving debugging and build reliability. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
7f7e745    to
    4698a8f      
    Compare
  
    
Enhances logEsbuildMessages to throw on critical errors by default,
ensuring build failures are properly surfaced instead of being silently logged.
Changes:
This prevents builds from appearing successful when esbuild encounters
critical errors, improving debugging and build reliability.
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]