fix: make App Hosting emulator startup and configuration errors non-blocking - #10951
fix: make App Hosting emulator startup and configuration errors non-blocking#10951christhompsongoogle wants to merge 1 commit into
Conversation
…locking ### Description Prevent missing configurations and startup failures for the App Hosting emulator from crashing emulators:start and blocking the rest of the emulator suite: - Check for package manager lockfiles (package-lock.json, yarn.lock, pnpm-lock.yaml) in shouldStart when startCommand is unconfigured, skipping startup with an error log if none are found. - Gracefully handle App Hosting emulator startup failure in startAll by cleaning up the failed instance and logging the error without crashing other emulators. Fixes b/546204399 ### Scenarios Tested - Unit tests in controller.spec.ts for App Hosting shouldStart handling (missing lockfile, startCommand configured, and lockfile present). ### Sample Commands - firebase init emulators (select all emulators) - firebase emulators:start
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces graceful handling of App Hosting emulator startup failures and auto-detection of start commands from lockfiles (such as package-lock.json, yarn.lock, or pnpm-lock.yaml) when no explicit start command is configured. This prevents startup failures from blocking other emulators. The review feedback suggests improving the unit tests by configuring the fs.existsSync stub to fall back to its original implementation for non-matching arguments to avoid potential side effects during test execution.
| const existsStub = sinon.stub(fs, "existsSync"); | ||
| existsStub.withArgs(sinon.match(/package-lock\.json/)).returns(true); |
There was a problem hiding this comment.
Stubbing a global/module-level method like fs.existsSync without a fallback can cause unexpected side effects or break other unstubbed calls during test execution. It is safer to configure the stub to fall back to the original implementation for non-matching arguments.
const existsStub = sinon.stub(fs, "existsSync");
existsStub.callsFake(existsStub.wrappedMethod);
existsStub.withArgs(sinon.match(/package-lock\.json/)).returns(true);
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio. |
Description
Prevent missing configurations and startup failures for the App Hosting emulator from crashing emulators:start and blocking the rest of the emulator suite:
Fixes b/546204399
Scenarios Tested
Sample Commands