fix: stop the health badge going green on a host that isn't a gateway - #10
Merged
Conversation
Two bugs compounding into a false "reachable · 4ms", found while verifying
the previous fix. Served on any loopback port outside 3000/3002, Wingman
defaulted its own Base URL to its own origin:
default base URL = http://localhost:44321 <- Wingman itself
health badge = reachable · 4ms <- green
`defaultBaseUrl` passed the current port through, and `checkHealth`
trusted `res.ok` alone — so Wingman's SPA fallback answered the probe with
`200 index.html` and the badge called it healthy. Sending then POSTs the
user's key at a static file server.
Neither half was reachable enough to matter when Wingman only ran on 3002.
It is now: the Manifest dashboard's dev drawer proxies Wingman onto an
arbitrary loopback port, so the passthrough became the common case.
- `defaultBaseUrl` targets the Manifest dev gateway (:3001) from any
loopback origin and falls back to Manifest Cloud when Wingman is itself
on :3001. It can no longer return its own origin.
- `checkHealth` rejects an HTML body — by content-type, and by sniffing a
leading `<` for servers that mislabel it. Anything fronted by a
single-page app or a catch-all reverse proxy answers every path with
`200 text/html`; that is not a gateway, and the badge now says so.
A non-JSON, non-HTML body is still accepted rather than demanding a
schema Manifest happens to emit.
Moving `defaultBaseUrl` next to the other base-URL logic makes it testable
without a DOM: it takes an optional location so the port cases can be
asserted directly. 17 new tests (77 total).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #9, found while verifying it. Two bugs compounding into a green badge pointing at nothing. Measured on the merged
main, Wingman served onlocalhost:44321:defaultBaseUrl()passed the current port straight through for anything other than 3000/3002, so Wingman defaulted to its own origin.checkHealththen trustedres.okalone — and Wingman's SPA fallback answers/api/v1/healthwith200 index.html, so the probe called it healthy. Hitting Send POSTs the user's API key at a static file server.Neither half mattered much when Wingman only ran on 3002. It does now: mnfst/manifest#2579 proxies Wingman onto an arbitrary loopback port for the dev drawer, so the passthrough went from edge case to common case.
Changes
defaultBaseUrltargets the Manifest dev gateway (:3001) from any loopback origin, and falls back to Manifest Cloud when Wingman is itself on:3001. It can no longer return its own origin.checkHealthrejects an HTML body — bycontent-type, and by sniffing a leading<for servers that mislabel it. Anything behind a single-page app or a catch-all reverse proxy answers every path with200 text/html; that isn't a gateway. A non-JSON, non-HTML body is still accepted rather than demanding the exact schema Manifest happens to emit.defaultBaseUrlmoves next to the other base-URL logic and takes an optional location, so the port cases are assertable without a DOM.After
Verification
17 new tests (77 total). Re-ran the full suites from #9 against this change: 16/16 base-URL matrix, 9/9 drawer matrix (streaming, all three wire formats, every previously-404ing URL shape), zero console errors. Verified in the real browser on the production bundle, not just the dev server.