Fix Extension Host crash when there is no current project - #956
Open
1fanwang wants to merge 1 commit into
Open
Conversation
`Container.currentProject` stays undefined until `showStatusBar` resolves a project for the open folder, so the side bar decoration provider dereferenced undefined and threw `Cannot read properties of undefined (reading 'rootPath')` on every decoration request for the `projectManager-view` scheme. Because the provider runs once per tree item, the errors flooded the Extension Host until it terminated. Guard the current project before comparing its root path, and cover the provider with tests for both the set and unset cases. Fixes alefragnani#938 Signed-off-by: 1fanwang <1fannnw@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an Extension Host crash in the Project Manager sidebar decoration provider by safely handling cases where no current project is set, and adds a focused test suite to prevent regressions.
Changes:
- Add a runtime guard in the sidebar decoration provider to avoid dereferencing
Container.currentProjectwhen it’s unset. - Add a new Mocha test suite that captures the actual registered
FileDecorationProviderand verifies behavior for: current project decorated, non-current not decorated, other schemes ignored, and “no current project” does not throw / does not decorate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/sidebar/decoration.ts | Prevents crashes by guarding access to Container.currentProject before reading rootPath. |
| src/test/suite/decoration.test.ts | Adds regression coverage for the sidebar decoration provider, including the “no current project” scenario that caused Extension Host crashes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The side bar decoration provider dereferences
Container.currentProjectwithout checking it, so it throwsCannot read properties of undefined (reading 'rootPath')whenever no current project is set. It runs once per tree item, so the errors flood the Extension Host until it terminates and Extension Bisect kicks in.Fixes #938
Container._currentProjectis declared but never initialized, and is only assigned fromshowStatusBar, which returns no value in four cases. Only the last needs an unmanaged folder:projectManager.showProjectNameInStatusBarisfalse, so it never resolves even when the folder is a saved projectprojectNamestrictNullChecksis not enabled, so typing the getterProject | undefinedwould not be enforced at the call site. The runtime guard is what fixes it.Prerequisites Checklist
masterbranchRegular PR
Changes Made
Container.currentProjectbefore comparing its root path, as suggested in the issue.SideBar Decoration Testscovering the unset current project plus existing behavior (current project decorated, others not, foreign schemes ignored). They interceptwindow.registerFileDecorationProviderto exercise the provider the extension actually registers, not a copy that could drift.Testing
npm testlaunches a real VS Code 1.130.0 via@vscode/test-electron. Not run on Windows, Linux or Remotes; the change is platform independent and does not touch the remote path.The two new tests fail on
masterand pass with the fix. The pre-fix failure comes from the product fileout/src/sidebar/decoration.js:15, matching the stack in the issue.Raw logs: before the fix (2 failing)
Raw logs: after the fix (68 passing)
Raw logs: Extension Host renderer log, same file across both runs
npm run lintreports 0 errors; the changed files add no new warnings.Documentation
Not applicable. This is a crash fix with no user-facing surface change.
Additional Notes
The guard is deliberately minimal, matching the suggestion in the issue. The other option, an
undefined-typed getter checked at the call site, only holds oncestrictNullChecksis enabled.