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

fix(vscode): recovers server process if terminated #850

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Conversation

tdstein
Copy link
Collaborator

@tdstein tdstein commented Jan 19, 2024

Screen.Recording.2024-01-19.at.2.51.43.PM.mov

Intent

Modifies the extension flow to restart the server if necessary before displaying the UI. This is accomplished by checking if...

  • The terminal that started the server is no longer present.
  • The port that the server is running on is no longer responsive.

Resolves #825
Resolves #846

Type of Change

    • Bug Fix
    • New Feature
    • Breaking Change
    • Documentation
    • Refactor
    • Tooling

Approach

Automated Tests

Directions for Reviewers

Checklist

Comment on lines +20 to +22
"activationEvents": [
"onStartupFinished"
],
Copy link
Collaborator Author

@tdstein tdstein Jan 19, 2024

Choose a reason for hiding this comment

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

The activate method is invoked when the startup is finished.

export async function activate(context: vscode.ExtensionContext) {

Comment on lines +15 to +17
const port = await ports.acquire();
service = new Service(port);
await service.start(context);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On the activation event, the service is started, which handles starting the server.

Comment on lines -10 to -23
export const ping = async (port: number, timeout: number = 30 * 1000): Promise<boolean> => {
try {
await wait({
resources: [
`http-get://${HOST}:${port}`
],
timeout: timeout
});
return true;
} catch (e) {
console.warn("failed waiting for port", e);
return false;
}
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to

private async isRunning(): Promise<boolean> {
try {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Most of this implementation is pulled from assistant.ts, which has been deleted, with a few refactors. Notably, the panel is now managed by services.ts. servers.ts and panels.ts can be thought of as the backend/frontend.


class StateManager {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed all of the StateManager code since I now understand how VSCode manages the extension lifecycle. The implementation initially existed as both a safety-check and a state logging facility.

@tdstein tdstein merged commit 7869b58 into main Jan 24, 2024
22 checks passed
@tdstein tdstein deleted the tdstein/825 branch January 24, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants