fix(azure): prevent azure-swa runtime errors, and response body returning {} #4195
Open
Xiang-CH wants to merge 6 commits intonitrojs:mainfrom
Open
fix(azure): prevent azure-swa runtime errors, and response body returning {} #4195Xiang-CH wants to merge 6 commits intonitrojs:mainfrom
Xiang-CH wants to merge 6 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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDocumentation switched examples to the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
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>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/presets/azure/runtime/_utils.ts`:
- Around line 35-61: The resolveBaseUrl function has misplaced/extra braces
causing a syntax error and an unreachable/default return; fix the function body
(function resolveBaseUrl(req: HttpRequest)) by removing the stray closing
brace(s) so the entire logic is inside the function, ensure every conditional
path returns a string (use forwardedProto || "http" when building candidate,
keep the try/catch for new URL(candidate).origin and for new
URL(originalUrl).origin), and add a final return "http://localhost" at the end
of resolveBaseUrl so callers like new URL(url, resolveBaseUrl(req)) never
receive undefined.
🪄 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: ead93783-8a95-4e76-81cc-4c71f18b410a
📒 Files selected for processing (1)
src/presets/azure/runtime/_utils.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/presets/azure/runtime/_utils.ts (1)
54-56: Avoid silent fallback for invalidx-ms-original-url.Line 54 currently swallows parse failures without a warning. Please log this recoverable invalid state (and remove the inline explanatory comment).
Proposed patch
const originalUrl = req.headers["x-ms-original-url"]; if (originalUrl) { try { return new URL(originalUrl).origin; - } catch { - // ignore invalid original URL + } catch (error) { + console.warn("[nitro] Invalid Azure SWA original URL", { + originalUrl, + error, + }); } }As per coding guidelines "Prefer explicit errors over silent failures; use warnings for recoverable situations and throw for invalid states".
🤖 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 54 - 56, The catch block that swallows parse failures for the "x-ms-original-url" header in src/presets/azure/runtime/_utils.ts should not be silent; replace the empty catch with a warning log that includes the invalid header value and the caught error (e.g., use the module's logger or console.warn to log `x-ms-original-url` and error), then continue the fallback logic, and remove the inline explanatory comment; ensure the log message is concise and clearly identifies the header and parse failure so recoverable invalid states are visible during debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/presets/azure/runtime/_utils.ts`:
- Around line 54-56: The catch block that swallows parse failures for the
"x-ms-original-url" header in src/presets/azure/runtime/_utils.ts should not be
silent; replace the empty catch with a warning log that includes the invalid
header value and the caught error (e.g., use the module's logger or console.warn
to log `x-ms-original-url` and error), then continue the fallback logic, and
remove the inline explanatory comment; ensure the log message is concise and
clearly identifies the header and parse failure so recoverable invalid states
are visible during debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c5fb078-90de-43ce-b2a3-e1d7c580e142
📒 Files selected for processing (1)
src/presets/azure/runtime/_utils.ts
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