fix: slow startup if no network is available #5055
Conversation
stfsy
changed the title
| @@ -45,6 +46,7 @@ export function getFunctionsSDKVersion(sourceDir: string): string | void { | |||
| const child = spawn.sync("npm", ["list", "firebase-functions", "--json=true"], { | |||
| cwd: sourceDir, | |||
| encoding: "utf8", | |||
| timeout: NPM_COMMAND_TIMEOUT_MILLIES, | |||
There was a problem hiding this comment.
We actually removed this spawn in a recent commit, do you mind pulling in the changes from master?
There was a problem hiding this comment.
@colerogers thank you very much, rebased from master
To prevent slow startups if no network is available
| @@ -16,6 +16,7 @@ interface NpmShowResult { | |||
| } | |||
|
|
|||
| const MIN_SDK_VERSION = "2.0.0"; | |||
| const NPM_COMMAND_TIMEOUT_MILLIES = 3000; | |||
There was a problem hiding this comment.
We discussed this internally and think that 10000 would be a more appropriate timeout for customers with slower connectivity or an intermittent issue
There was a problem hiding this comment.
@colerogers ok good point. I updated the value as per your recommendation
|
@colerogers anything I need to? any more prerequisites for it getting merged? ☺ |
Codecov ReportBase: 56.23% // Head: 56.24% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #5055 +/- ##
==========================================
+ Coverage 56.23% 56.24% +0.01%
==========================================
Files 317 317
Lines 21409 21410 +1
Branches 4366 4366
==========================================
+ Hits 12040 12043 +3
+ Misses 8319 8318 -1
+ Partials 1050 1049 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@colerogers Thanks for merging! |
Preventing slow startups (> 60s) if no network is available. Tried to pick a reasonable timeout. Currently from Barcelona the NPM commands take 500ms to return.
Description
Fixes #5054
Scenarios Tested
Sample Commands