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

feat: use new proxy 🥳 fix #1396 #1703

Merged
merged 41 commits into from May 21, 2024
Merged

feat: use new proxy 🥳 fix #1396 #1703

merged 41 commits into from May 21, 2024

Conversation

marclave
Copy link
Member

@marclave marclave commented May 12, 2024

This PR migrates to the new proxy, which is now hosted on https://proxy.scalar.com

We used to have a custom proxy, which returned JSON always. The new proxy is a raw HTTP proxy (written in Go), which expects to get a ?scalar_url= query parameter, everything else (headers and such) aren’t touched at all and just proxied through.

If you’re using the old proxy (https://api.scalar.com/request-proxy) we magically replace it with the new URL (https://proxy.scalar.com) on the fly. You’ll receive a deprecation warning, though. Please update your configuration to use the new proxy URL.

If you used to self-host our @scalar/api-client-proxy we kindly ask you to switch to our new proxy (examples/proxy-server) written in Go. If you need help with that, reach out to us on Discord.

Additional improvements

  • the proxy is skipped for requests to localhost, 127.0.0.1 and ::1
  • Network errors (eg. domain doesn’t exist) are shown
  • Not-JSON content is correctly formatted again
  • CORS errors at least show "ERR_NETWORK" in the response body
  • some more tests
  • more lines deleted than added

Additional fixes

Preview

Screenshot 2024-05-15 at 16 11 58 Screenshot 2024-05-15 at 16 12 14 Screenshot 2024-05-15 at 16 25 23

(Comment hijacked by @hanspagel)

Copy link

changeset-bot bot commented May 12, 2024

🦋 Changeset detected

Latest commit: 3ec01d2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@scalar/api-reference Patch
@scalar/play-button Patch
@scalar/api-client Patch
@scalar/oas-utils Patch
@scalar/nuxt Patch
@scalar/cli Patch
@scalar/api-reference-react Patch
@scalar/express-api-reference Patch
@scalar/fastify-api-reference Patch
@scalar/hono-api-reference Patch
@scalar/nestjs-api-reference Patch
@scalar/nextjs-api-reference Patch
@scalar/api-client-react Patch
@scalar/components Patch
@scalar/mock-server Patch
@scalar/docusaurus Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@hanspagel hanspagel marked this pull request as draft May 13, 2024 10:53
@hanspagel

This comment was marked as resolved.

@marclave marclave marked this pull request as ready for review May 14, 2024 16:39
Copy link

relativeci bot commented May 15, 2024

#1643 Bundle Size — 1.91MiB (+0.05%).

3ec01d2(current) vs ef0c984 main#1642(baseline)

Warning

Bundle contains 4 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Regression 1 regression
                 Current
#1643
     Baseline
#1642
Regression  Initial JS 1.91MiB(+0.05%) 1.91MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 100% 0%
No change  Chunks 1 1
No change  Assets 1 1
No change  Modules 1082 1082
No change  Duplicate Modules 0 0
No change  Duplicate Code 0% 0%
No change  Packages 157 157
No change  Duplicate Packages 4 4
Bundle size by type  Change 1 change Regression 1 regression
                 Current
#1643
     Baseline
#1642
Regression  JS 1.91MiB (+0.05%) 1.91MiB

Bundle analysis reportBranch marc/migrate-to-new-proxyProject dashboard

@hanspagel hanspagel changed the title feat: use new proxy 🥳 feat: use new proxy 🥳 fix #1396 May 15, 2024
@hanspagel hanspagel self-assigned this May 21, 2024
Copy link
Member

@hanspagel hanspagel left a comment

Choose a reason for hiding this comment

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

LGTM, wdyt @marclave and others?

Copy link
Contributor

@geoffgscott geoffgscott left a comment

Choose a reason for hiding this comment

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

Looks good.

Copy link
Contributor

@amritk
Copy link
Member

amritk commented May 21, 2024

The code looks fine, but I tried using the preview and it says 201 but I got a CORS error? No idea whats going on lol
image
image

Copy link
Member

@amritk amritk left a comment

Choose a reason for hiding this comment

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

Tested locally and works great! The issue is with the deploy preview

@hanspagel
Copy link
Member

The cors header has *, * as a value! I fixed this in the Go proxy, but proxy.scalar.com doesn’t have the fix yet. Because it’s in this PR here. 😂

Copy link
Member Author

@marclave marclave left a comment

Choose a reason for hiding this comment

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

HUGE! let's goooo

@marclave marclave merged commit 5aa656f into main May 21, 2024
8 checks passed
@marclave marclave deleted the marc/migrate-to-new-proxy branch May 21, 2024 20:39
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.

None yet

5 participants