Skip to content
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

Add timeout to avoid long-term hangs #2546

Merged
merged 8 commits into from
Nov 10, 2023
Merged

Conversation

jamonholmgren
Copy link
Member

We've had multiple issues over the years with Ignite hanging forever and people (patiently!) waiting for hours for it to finish.

Examples: #1249, #2084, #1735, #1365, #1287, #172, and many more.

This adds a simple timeout that won't allow Ignite to take longer than 10 minutes once we start spinning up a new app.

This might be a bit too aggressive; I don't know what our longest installs take these days, but I just tried one using npm as the package manager and choosing the DIY option on Ignite v9 and it took roughly 72 seconds, which is 12% of this timeout.

This does NOT need to be merged prior to v9, but can be; it's a relatively small change.

Copy link
Contributor

@frankcalise frankcalise left a comment

Choose a reason for hiding this comment

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

Looking good! Left some comments.

Will also time an install with npm/yarn on an older intel machine to see how the 10 minutes buffer is (should be fine, though).

bun.lockb Outdated Show resolved Hide resolved
.circleci/config.yml Show resolved Hide resolved
@frankcalise
Copy link
Contributor

@jamonholmgren one way to avoid the timer being too aggressive, maybe a cli flag to disable it for certain situations? like terrible car/plane wifi or something?

@jamonholmgren
Copy link
Member Author

Added a --no-timeout flag!

@jamonholmgren
Copy link
Member Author

@frankcalise I'll let you merge when you're ready, if you're ready!

src/commands/new.ts Outdated Show resolved Hide resolved
@frankcalise frankcalise merged commit ca36ac7 into feat/rn-72-expo-49 Nov 10, 2023
1 check passed
@frankcalise frankcalise deleted the fix/long-delay branch November 10, 2023 02:34
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