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

Extension fails to initialize when server.* properties are modified in Vite(st) configuration #226

Closed
MilanKovacic opened this issue Feb 15, 2024 · 4 comments
Labels
p4-important Violate documented behavior or significantly improves performance (priority)

Comments

@MilanKovacic
Copy link
Collaborator

MilanKovacic commented Feb 15, 2024

Modifying the following server properties makes the extension broken:

  1. server.https set to true
  2. server.port changed
@MilanKovacic MilanKovacic added the p3-minor-bug An edge case that only affects very specific usage (priority) label Feb 15, 2024
@kamikazePT
Copy link

Workaround, check if mode === 'test' to opt-out the plugin

@saitonakamura
Copy link
Contributor

Any https configuration in server seems to affect this. Also there are similar reports scattered across other issues
#196 (comment)
#146 (comment)

I researched it a bit and it seems that multiple parts are at play here. I may be mistaken in any of them though

tldr

The issue looks very similar to https://github.com/vitest-dev/vitest/pull/4855/files but extension currently has no way to know whether api is secure or not

vitest part

It all starts with the fact that dev server in configureServer hook of VitestPlugin will inherit https settings from config.server.https
https://github.com/vitest-dev/vitest/blob/a51467af718513f704128171d20cd54b33b1262a/packages/vitest/src/node/vite-plugin.ts#L43
I suspect that related code of how settings are inherit will be in the vite source itself, cause I wasn't able to understand where it gets passed in vitest code

This https-configured server then gets saved in vitest context
https://github.com/vitest-dev/vitest/blob/b9d1a97599566fc07089d9f92d9388193a3dcf19/packages/vitest/src/node/plugins/index.ts#L224

And then when the api setup is called
https://github.com/vitest-dev/vitest/blob/b9d1a97599566fc07089d9f92d9388193a3dcf19/packages/vitest/src/node/plugins/index.ts#L226
it takes the server from the context
https://github.com/vitest-dev/vitest/blob/b8140fcadc114fd2d05ca997da4f7aa4e226417d/packages/vitest/src/api/setup.ts#L27
and configures websocket connection upgrade

But then does not respect this https in the logs here
https://github.com/vitest-dev/vitest/blob/ffa358517eb6a5ae6c975495d9ac3a05bfd9939f/packages/vitest/src/node/logger.ts#L101

vscode extension part

Extension run vitest api to get the config via executing it with cli api args

[...workspace.args, '--api.port', port.toString(), '--api.host', '127.0.0.1'],

and then assumes that it's a non-secure url

{ port, url = `ws://localhost:${port}/__vitest_api__`, reconnectInterval, reconnectTries }: {

which is not true and this is why connection just fails

additional challenge is if https is configured with a self-signed certificate just using wss:// is not enough, it's also needed to properly configure websocket client to ignore this
new WebSocketConstructor(url, { rejectUnauthorized: false }))

ws: shallowReactive(new WebSocketConstructor(url)),

results

I was able to make this work by explicitly using wss:// protocol and rejectUnauthorized: false. But I currently don't see any trivial how extension can understand whether it needs to use secure or non-secure url because it needs it to get the config (which could theoretically contain this information, not sure if does, though)

possible solutions

force non-secure for vitest api

The fact that vite config https settings are leaking for vitest api/ui seems kinda counterintuitive for me and wonder if it should be the case. What problems may arise if vitest api will be made non-secure all the time?

allow to opt out of secure vitest api

Something like vitest --api --https=false which extension can take advantage of.

try both wss and ws (and maybe even rejectUnauthorized: false)

Just try multiple variants of connecting and see which one works. This looks like a last resort for me.

All of those may be related to the idea of splitting vitest api for different usecases vitest-dev/vitest#3138

cc @sheremet-va

@MilanKovacic MilanKovacic changed the title Extension fails to initialize when basicSsl plugin is used Extension fails to initialize when server.* properties are modified in Vite(st) configuration Feb 24, 2024
@sheremet-va sheremet-va added p4-important Violate documented behavior or significantly improves performance (priority) and removed p3-minor-bug An edge case that only affects very specific usage (priority) labels Feb 26, 2024
@sheremet-va
Copy link
Member

The current idea is to use Vitest API directly: #253

With this, we don't need to start a server with a user configuration at all.

@sheremet-va
Copy link
Member

This is fixed in pre-release 0.5.0 and higher. Note that the extension now requires Vitest 1.4.0 or higher.

We no longer start any server.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
Development

No branches or pull requests

4 participants