fix(azure): prevent azure-swa runtime errors, and response body returning {} #4195
Open
Xiang-CH wants to merge 4 commits intonitrojs:mainfrom
Open
fix(azure): prevent azure-swa runtime errors, and response body returning {} #4195Xiang-CH wants to merge 4 commits intonitrojs:mainfrom
Xiang-CH wants to merge 4 commits intonitrojs:mainfrom
Conversation
…a` preset and runtime configurations
|
@Xiang-CH is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRenamed Azure preset to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/2.deploy/20.providers/azure.md (1)
30-40:⚠️ Potential issue | 🟡 MinorAlign this section with the actual preset/runtime selection logic.
Line 32 still says this file is generated for the
azurepreset, but this page andsrc/presets/azure/preset.ts:1-27are aboutazure-swa. Also,src/presets/azure/utils.ts:6-23only consultsprocess.versions.nodewhen readingpackage.jsonfails; ifengines.nodeexists but is anything other than the exact strings"20"or"22", the generated config stays onnode:18.Proposed fix
-Nitro automatically generates this configuration file whenever the application is built with the `azure` preset. +Nitro automatically generates this configuration file whenever the application is built with the `azure-swa` preset. @@ -| **[platform.apiRuntime](https://learn.microsoft.com/en-us/azure/static-web-apps/configuration#platform)** | Uses `node:20` or `node:22` when `package.json` `engines.node` (or your current Node major version) matches those supported versions; otherwise falls back to `node:18`. | `node:18` | +| **[platform.apiRuntime](https://learn.microsoft.com/en-us/azure/static-web-apps/configuration#platform)** | Uses `node:20` or `node:22` when `package.json` `engines.node` is exactly `20` or `22`. If Nitro cannot read that value, it falls back to your current Node major when it is `20` or `22`; otherwise it uses `node:18`. | `node:18` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/2.deploy/20.providers/azure.md` around lines 30 - 40, Update the docs text to reflect the actual preset and runtime-selection logic: change the preset name from `azure` to `azure-swa` (referencing src/presets/azure/preset.ts) and clarify Node runtime selection behavior as implemented in src/presets/azure/utils.ts — i.e., Nitro reads package.json.engines.node and will select `node:20` or `node:22` only when the engines.node string exactly equals "20" or "22", otherwise it falls back to `node:18`; also note that process.versions.node is only consulted if reading package.json fails. Ensure the table entry for platform.apiRuntime and any surrounding text reflect this exact behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/2.deploy/20.providers/azure.md`:
- Line 9: The :read-more shortcode target is malformed; update the
:read-more{...} invocation (the line containing :read-more{title="Azure Static
Web Apps"
to="[https://azure.microsoft.com/en-us/products/app-service/static"}](https://azure.microsoft.com/en-us/products/app-service/static"})
to use a proper to="..." attribute without stray brackets or trailing link
syntax so the to value is exactly the Azure URL (i.e., fix the to attribute
inside the :read-more shortcode).
In `@src/presets/azure/runtime/_utils.ts`:
- Around line 35-48: The resolveBaseUrl function currently returns
`${forwardedProto || "http"}://${host}` directly from untrusted headers;
instead, construct that candidate URL from forwardedProto/host, attempt to parse
it with new URL(...) and only return candidate.origin if parsing succeeds; if
parsing fails, fall through to check x-ms-original-url (parse and return its
origin if valid) and finally return a safe default like "http://localhost";
update references in resolveBaseUrl and ensure headers "x-forwarded-proto",
"x-forwarded-host", and "host" are validated before returning to avoid passing a
malformed base to new URL in azure-swa.ts.
---
Outside diff comments:
In `@docs/2.deploy/20.providers/azure.md`:
- Around line 30-40: Update the docs text to reflect the actual preset and
runtime-selection logic: change the preset name from `azure` to `azure-swa`
(referencing src/presets/azure/preset.ts) and clarify Node runtime selection
behavior as implemented in src/presets/azure/utils.ts — i.e., Nitro reads
package.json.engines.node and will select `node:20` or `node:22` only when the
engines.node string exactly equals "20" or "22", otherwise it falls back to
`node:18`; also note that process.versions.node is only consulted if reading
package.json fails. Ensure the table entry for platform.apiRuntime and any
surrounding text reflect this exact behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0dfd57d8-e0cc-49e1-acde-3002e1923754
📒 Files selected for processing (3)
docs/2.deploy/20.providers/azure.mdsrc/presets/azure/runtime/_utils.tssrc/presets/azure/runtime/azure-swa.ts
Comment on lines
+35
to
+48
| export function resolveBaseUrl(req: HttpRequest) { | ||
| const forwardedProto = req.headers["x-forwarded-proto"]; | ||
| const forwardedHost = req.headers["x-forwarded-host"]; | ||
| const host = forwardedHost || req.headers["host"]; | ||
| if (host) { | ||
| return `${forwardedProto || "http"}://${host}`; | ||
| } | ||
| const originalUrl = req.headers["x-ms-original-url"]; | ||
| if (originalUrl) { | ||
| try { | ||
| return new URL(originalUrl).origin; | ||
| } catch { | ||
| // ignore invalid original URL | ||
| } |
Contributor
There was a problem hiding this comment.
Validate the forwarded origin before returning it.
x-forwarded-host / host are still external input here. Returning ${proto}://${host} as-is means new URL(url, resolveBaseUrl(req)) in src/presets/azure/runtime/azure-swa.ts can still throw on malformed headers, which brings back the runtime failure this patch is trying to eliminate. Parse that candidate here and only return .origin after it validates; otherwise continue to x-ms-original-url / localhost.
Proposed fix
export function resolveBaseUrl(req: HttpRequest) {
const forwardedProto = req.headers["x-forwarded-proto"];
const forwardedHost = req.headers["x-forwarded-host"];
const host = forwardedHost || req.headers["host"];
if (host) {
- return `${forwardedProto || "http"}://${host}`;
+ const candidate = `${forwardedProto || "http"}://${host}`;
+ try {
+ return new URL(candidate).origin;
+ } catch (error) {
+ console.warn("[nitro] Invalid Azure SWA forwarded origin", {
+ candidate,
+ error,
+ });
+ }
}
const originalUrl = req.headers["x-ms-original-url"];As per coding guidelines "Prefer explicit errors over silent failures; use warnings for recoverable situations and throw for invalid states".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function resolveBaseUrl(req: HttpRequest) { | |
| const forwardedProto = req.headers["x-forwarded-proto"]; | |
| const forwardedHost = req.headers["x-forwarded-host"]; | |
| const host = forwardedHost || req.headers["host"]; | |
| if (host) { | |
| return `${forwardedProto || "http"}://${host}`; | |
| } | |
| const originalUrl = req.headers["x-ms-original-url"]; | |
| if (originalUrl) { | |
| try { | |
| return new URL(originalUrl).origin; | |
| } catch { | |
| // ignore invalid original URL | |
| } | |
| export function resolveBaseUrl(req: HttpRequest) { | |
| const forwardedProto = req.headers["x-forwarded-proto"]; | |
| const forwardedHost = req.headers["x-forwarded-host"]; | |
| const host = forwardedHost || req.headers["host"]; | |
| if (host) { | |
| const candidate = `${forwardedProto || "http"}://${host}`; | |
| try { | |
| return new URL(candidate).origin; | |
| } catch (error) { | |
| console.warn("[nitro] Invalid Azure SWA forwarded origin", { | |
| candidate, | |
| error, | |
| }); | |
| } | |
| } | |
| const originalUrl = req.headers["x-ms-original-url"]; | |
| if (originalUrl) { | |
| try { | |
| return new URL(originalUrl).origin; | |
| } catch { | |
| // ignore invalid original URL | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/presets/azure/runtime/_utils.ts` around lines 35 - 48, The resolveBaseUrl
function currently returns `${forwardedProto || "http"}://${host}` directly from
untrusted headers; instead, construct that candidate URL from
forwardedProto/host, attempt to parse it with new URL(...) and only return
candidate.origin if parsing succeeds; if parsing fails, fall through to check
x-ms-original-url (parse and return its origin if valid) and finally return a
safe default like "http://localhost"; update references in resolveBaseUrl and
ensure headers "x-forwarded-proto", "x-forwarded-host", and "host" are validated
before returning to avoid passing a malformed base to new URL in azure-swa.ts.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/2.deploy/20.providers/azure.md (1)
40-40: Clarify that navigationFallback.rewrite is a default, not absolute.The documentation states that
navigationFallback.rewrite"Is always/api/server", but the implementation shows this is the generated default value that can be overridden viaazure.config.navigationFallback(see context snippet 3). Consider rephrasing to "Defaults to/api/server" for accuracy.📝 Suggested clarification
-| **[navigationFallback.rewrite](https://learn.microsoft.com/en-us/azure/static-web-apps/configuration#fallback-routes)** | Is always `/api/server` | `/api/server` | +| **[navigationFallback.rewrite](https://learn.microsoft.com/en-us/azure/static-web-apps/configuration#fallback-routes)** | Defaults to `/api/server` | `/api/server` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/2.deploy/20.providers/azure.md` at line 40, Update the docs table entry that currently reads "Is always `/api/server`" for navigationFallback.rewrite to indicate it's a generated default (e.g., "Defaults to `/api/server`") and mention that it can be overridden via azure.config.navigationFallback (referencing the symbol navigationFallback.rewrite and azure.config.navigationFallback to locate the text to edit).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/2.deploy/20.providers/azure.md`:
- Line 32: Update the documentation text that says "azure preset" to use the
correct preset name "azure-swa": locate the sentence that reads "built with the
`azure` preset" and change the identifier to `azure-swa` so it matches the rest
of the file and the actual preset definition.
---
Nitpick comments:
In `@docs/2.deploy/20.providers/azure.md`:
- Line 40: Update the docs table entry that currently reads "Is always
`/api/server`" for navigationFallback.rewrite to indicate it's a generated
default (e.g., "Defaults to `/api/server`") and mention that it can be
overridden via azure.config.navigationFallback (referencing the symbol
navigationFallback.rewrite and azure.config.navigationFallback to locate the
text to edit).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3144c67-2d97-45b2-81e4-1c9e48a03dc7
📒 Files selected for processing (1)
docs/2.deploy/20.providers/azure.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
🔗 Linked issue
#2590
❓ Type of change
📚 Description
This PR improves the azure-swa preset so Nitro runs reliably on Azure Static Web Apps
What was failing
How it is fixed
response.body(a stream) withBuffer.from(await response.arrayBuffer())when a body exists, so Azure Functions gets bytes instead of a stream object that could surface as{}or otherwise fail at runtime.headers: new Headers(req.headers)so the FetchRequestreceives the same headers as the Azure HTTP triggerURL(url, resolveBaseUrl(req))instead of a relative URL string. resolveBaseUrl prefersx-forwarded-proto / x-forwarded-host(or host), then falls back to the origin from x-ms-original-url, thenhttp://localhost.📝 Checklist