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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Make UI Mode compatible with subpath proxying #29564

Closed
bockstaller opened this issue Feb 19, 2024 · 2 comments
Closed

[Feature]: Make UI Mode compatible with subpath proxying #29564

bockstaller opened this issue Feb 19, 2024 · 2 comments
Assignees
Labels
feature-ui-mode open-to-a-pull-request The feature request looks good, we are open to reviewing a PR v1.43

Comments

@bockstaller
Copy link
Contributor

馃殌 Feature Request

UI-Mode supports test execution via docker and codespaces by starting it with the following command:

npx playwright test --ui-port=8080 --ui-host=0.0.0.0

The trace viewer is launched and a websocket connection to the test runner is established.

This breaks as soon as the port opened by playwright is proxied via a subpath instead of a subdomain because the websocket is only taking the domain, port and guid into account.

This mode isn't supported by codespaces, but other deployment methods for remote code instances support this https://coder.com/docs/code-server/latest/guide#using-a-subdomain this way of proxying.

Example

Following the relative URI resolution should allow a different way of creating the Websocket connection URL.

Ap possible implementation replacing this line could be.

const wsURL = URL(`../${guid}`, document.location))
wsURL.protocol = (window.location.protocol === 'https:' ? 'wss:' : 'ws:')
const ws = new WebSocket(wsURL)

This should keep backward compatibility while allowing subpath proxying.

Motivation

This change would make playwrights ui mode usable in situations where the devs influence on larger infrastructure is limited.
Especially whenever it isn't possible to proxy via subdomains.

@bockstaller bockstaller changed the title [Feature]: Make UI Mode compatible with subpaths [Feature]: Make UI Mode compatible with subpath proxying Feb 19, 2024
@yury-s yury-s added feature-ui-mode open-to-a-pull-request The feature request looks good, we are open to reviewing a PR labels Feb 20, 2024
@yury-s yury-s self-assigned this Feb 21, 2024
@yury-s yury-s added the v1.43 label Feb 21, 2024
bockstaller added a commit to bockstaller/playwright that referenced this issue Feb 22, 2024
should make ui mode compatible with subpath proxying by using relative
instead of absolute paths as discussed in microsoft#29564
@bockstaller
Copy link
Contributor Author

@yury-s I created a PR making the suggested changes. Happy to iterate on it

@bockstaller
Copy link
Contributor Author

There is a small bug in the original implementation I am addressing in this PR:

#29924

yury-s pushed a commit that referenced this issue May 20, 2024
This is a follow up #29564 

I did a deep dive on a redirect issue I observed in my infrastructure
and originally attributed to some configuration mistakes on my part.
I have code hosted on `example.com/code` and use subdomain proxying.
This leads to the uimode being exposed on
`example.com/code/proxy/{{port}}`.

Clicking on the open uimode link shown by vscode redirected with a 302
to `example.com/proxy/{{port}}`

The absolute redirect url overruled the relative path handling reverse
proxies rely on.

This PR turns the absolute into a relative url to avoid this issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-ui-mode open-to-a-pull-request The feature request looks good, we are open to reviewing a PR v1.43
Projects
None yet
Development

No branches or pull requests

2 participants