Skip to content

Conversation

@styfle
Copy link
Owner

@styfle styfle commented Mar 19, 2025

We give the npm package 25 seconds and that leaves 5 seconds to cleanup when we fail for a total of 30 seconds.

@styfle styfle requested a review from Copilot March 19, 2025 00:30
@vercel
Copy link

vercel bot commented Mar 19, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
packagephobia ✅ Ready (Inspect) Visit Preview Mar 19, 2025 0:30am

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR reduces the npm install timeout from 2 minutes to 25 seconds and updates the related functions to support aborting the process.

  • Introduces an AbortController in calculatePackageSize within npm-stats.ts.
  • Updates the npmInstall function signature in npm-wrapper.ts to accept an AbortSignal.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/util/backend/npm-stats.ts Uses AbortController to abort npm install and cleans up after the timeout.
src/util/backend/npm-wrapper.ts Modifies npmInstall to accept a new AbortSignal parameter for cancellation.
Comments suppressed due to low confidence (2)

src/util/backend/npm-stats.ts:36

  • Consider wrapping the subsequent cleanup call (execFileAsync) in a try-catch block to properly handle any errors that might occur during the abort process.
console.error(`Aborting npm install and deleting ${tmpPackage}`);

src/util/backend/npm-wrapper.ts:9

  • Ensure that all invocations of npmInstall are updated to pass an AbortSignal to prevent runtime errors when the new parameter is required.
export async function npmInstall(

@styfle styfle merged commit 10127fe into main Mar 19, 2025
4 checks passed
@styfle styfle deleted the styfle/fix-timeout branch March 19, 2025 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants