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

Setup proxy to host instead of rewriting URL #386

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

edoardopirovano
Copy link
Contributor

@edoardopirovano edoardopirovano commented Feb 6, 2024

This should unblock customers using Auth0 which will stop the page load if it sees a non-HTTPS and non-localhost URL.

Sample run on my personal website: https://github.com/edoardopirovano/website/actions/runs/7797651026/job/21267781609

Screenshots in the test run all look good: https://app.meticulous.ai/projects/edoardopirovano/website/test-runs/tnmzLBTmNqDJqQm8LKRpMzQbFd

Viewing a simulation confirms we were hitting localhost and using the proxy: https://app.meticulous.ai/projects/edoardopirovano/website/simulations/ggGWCJ6kNPnbmKbtGKqpqLDTq7

Resolves ENG-862

Copy link

vercel bot commented Feb 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
report-diffs-action ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 6, 2024 5:05pm

Copy link

github-actions bot commented Feb 6, 2024

✅ Meticulous spotted zero visual differences across 8 screens tested: view results.

Test suite: test Meticulous with deployment url. Last updated for commit 5c615d1. This comment will update as new commits are pushed.

Copy link

github-actions bot commented Feb 6, 2024

✅ Meticulous spotted zero visual differences across 8 screens tested: view results.

Test suite: test Meticulous with app url. Last updated for commit 5c615d1. This comment will update as new commits are pushed.

Copy link
Contributor

@Que3216 Que3216 left a comment

Choose a reason for hiding this comment

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

Nice!

@edoardopirovano edoardopirovano merged commit 1f5837e into main Feb 6, 2024
7 checks passed
@edoardopirovano edoardopirovano deleted the edoardo/proxy-to-host branch February 6, 2024 17:08
edoardopirovano added a commit that referenced this pull request Feb 6, 2024
edoardopirovano added a commit that referenced this pull request Feb 7, 2024
After #386,
this connection check no longer makes sense as we can always open a port
to the proxy so it isn't really checking anything. Instead, let's try
and actually fetch the appUrl and see if the server behind the proxy
gives us _something_ back. We don't check the status code other than 502
which is what the proxy will return if it can't connect to the server
since this most closely mimics the behaviour of just opening a socket
that we had before.

This PR also adds in some waiting, so that we'll retry a couple of times
to allow for the fact that the server to test against may take a bit of
time to spin up. This should avoid the need for customers to add/tweak
manual `sleep`s in their workflow.

Also, one small fix to an error message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants