Skip to content

Improve error handling during package installation#80

Open
realfakenerd wants to merge 2 commits into
zed-extensions:mainfrom
realfakenerd:feat/improve-lsp-error-handling
Open

Improve error handling during package installation#80
realfakenerd wants to merge 2 commits into
zed-extensions:mainfrom
realfakenerd:feat/improve-lsp-error-handling

Conversation

@realfakenerd

Copy link
Copy Markdown
Contributor

Description

This PR enhances the install_package_if_needed logic in src/svelte.rs to make the LSP installation process more robust and resilient to network or permission failures.

Previously, the installation logic was overly optimistic, potentially failing silently and leading to downstream crashes when trying to spawn the language server from a non-existent or corrupted directory.

Changes

  • Explicit Error Reporting: Replaced the silent failure approach with an explicit match statement on zed::npm_install_package. If the installation fails and no cached version is available, it now returns a descriptive critical error.
  • Integrity/Sanity Check: Added a post-installation check using fs::metadata to verify that the package_path directory actually exists on disk before marking the package as successfully installed.

@cla-bot cla-bot Bot added the cla-signed label Jun 9, 2026

@ghostdevv ghostdevv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hey, thanks for the PR! I'm curious what the motivation is? did this solve/help debug a tangible issue for you?

@realfakenerd

Copy link
Copy Markdown
Contributor Author

Hey man @ghostdevv! sorry for the delay, lot of things happening at the same time in my life. Well it was more of a preventive measure, while looking at the installation logic i noticed it was a bit optimistic and could fail silently. I wanted to make the process more robust. I am still learning rust and collaborating on github, so if i made something wrong, tell me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants