feat: Add Scaleway serverless function preset#3970
feat: Add Scaleway serverless function preset#3970Siilwyn wants to merge 9 commits intonitrojs:mainfrom
Conversation
|
@Siilwyn is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
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:
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)
📝 WalkthroughWalkthroughAdds Scaleway Serverless Functions support: new docs, preset registration and types, a Scaleway preset implementation with Rollup ESM output, a runtime handler translating Scaleway events to Nitro requests, a test suite, and a dev dependency. 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 |
Siilwyn
force-pushed
the
scaleway
branch
3 times, most recently
from
1b2a136 to
f8c6e3a
Compare
|
Moved from draft so CodeRabbit picks it up. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/presets/scaleway/runtime/scaleway-serverless.ts`:
- Around line 11-29: In handler, remove non-null assertions on event.headers and
event.queryStringParameters and guard them with safe fallbacks (e.g., const hdrs
= event.headers ?? {}; const qs = event.queryStringParameters ?? {}), iterate
hdrs safely when building headers map, and perform a case-insensitive lookup for
X-Forwarded-Proto (check hdrs["x-forwarded-proto"] and hdrs["X-Forwarded-Proto"]
or normalize keys) when composing the origin for joinURL; also handle body
decoding based on event.isBase64Encoded (decode to string/buffer when true) and
treat both GET and HEAD as having no body when constructing the Request. Ensure
you update references in handler to use these new locals (hdrs, qs, decodedBody)
and remove any remaining ! non-null assertions.
🧹 Nitpick comments (1)
docs/2.deploy/20.providers/scaleway.md (1)
1-7: Add a minimal config snippet for quicker onboarding.A short
nitro.configexample would make the preset immediately actionable for readers skimming the page.✍️ Example addition
# Scaleway > Deploy Nitro apps to Scaleway. **Preset:** `scaleway-serverless` +```ts +// nitro.config.ts +export default defineNitroConfig({ + preset: "scaleway-serverless", +}); +``` + :read-more{title="Scaleway Serverless Functions" to="https://www.scaleway.com/en/docs/serverless-functions/"}
Siilwyn
force-pushed
the
scaleway
branch
2 times, most recently
from
0a72c5c to
71d53b9
Compare
✅ Actions performedReviews resumed. |
✅ Actions performedFull review triggered. |
src/presets/scaleway/preset.ts
Outdated
| }, | ||
| }, | ||
| { | ||
| name: "scaleway-serverless" as const, |
There was a problem hiding this comment.
We might choose a more clearname for this, perhaps We might choose a better name perhaps scaleway-functions?
(since we also have Scaleway serverless containers product)
| headers.get("Host")!, | ||
| event.path | ||
| ), | ||
| event.queryStringParameters ?? {} |
There was a problem hiding this comment.
Can you please refactor to use https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams to stringify queryStringParameters? (also don't we have access to original request string?)
| import { useNitroApp } from "nitro/app"; | ||
| import { joinURL, withQuery } from "ufo"; | ||
| import type { serveHandler } from "@scaleway/serverless-functions"; | ||
| import { Buffer } from "node:buffer"; |
There was a problem hiding this comment.
I like adding these instead of relying on globals but all good, removed in 32ca17a
| import { defineNitroPreset } from "../_utils/preset.ts"; | ||
|
|
||
| const scalewayServerless = defineNitroPreset( | ||
| { |
There was a problem hiding this comment.
we might enable serve static handler (with inlined contents) or explain it in docs (containers have better support for static assets i guess also)
commit: |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/scaleway/runtime/scaleway-functions.ts`:
- Line 26: The assignment to const body using event.isBase64Encoded assumes
event.body is present and a decodable string; guard this by first checking
event.isBase64Encoded and that event.body is a non-empty string (or Buffer)
before calling Buffer.from, and if the body is missing or not a string throw a
clear Error mentioning event.isBase64Encoded and the missing/invalid event.body;
otherwise perform the base64 decode, e.g. only call Buffer.from(event.body,
"base64") when the guard passes, and keep the existing fallback to event.body
when not base64-encoded.
- Around line 13-15: The construction of the Headers object can throw when
event.headers is missing and will turn null/undefined values into the literal
strings "null"/"undefined"; update the const headers creation to use a safe
default and drop nullish entries by replacing Object.entries(event.headers!)
with Object.entries(event.headers ?? {}) and filter entries where value != null
before mapping to strings (e.g., keep only entries with value !== null && value
!== undefined), then map values to String(value) and pass the resulting object
into new Headers so no nullish header values are included.
- Around line 19-21: The code blindly uses headers.get("Host")! when building
the request URL (the expression near headers.get("X-Forwarded-Proto") === "http"
... headers.get("Host")! ... event.path), which can produce opaque downstream
failures; instead, read headers.get("Host") into a local (e.g., host) and if
it's null or empty throw a clear, actionable Error (for example "Missing Host
header in incoming request; cannot construct target URL") before continuing to
construct the URL—update the URL construction site (where headers.get("Host")!
is used) to use that validated host variable.
🪄 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: 826c3cc0-e789-4031-bdfc-478d3ce1fdc4
📒 Files selected for processing (5)
docs/2.deploy/20.providers/scaleway.mdsrc/presets/_types.gen.tssrc/presets/scaleway/preset.tssrc/presets/scaleway/runtime/scaleway-functions.tstest/presets/scaleway.test.ts
✅ Files skipped from review due to trivial changes (3)
- docs/2.deploy/20.providers/scaleway.md
- test/presets/scaleway.test.ts
- src/presets/scaleway/preset.ts
| const headers = new Headers( | ||
| Object.fromEntries(Object.entries(event.headers!).map(([key, value]) => [key, String(value)])) | ||
| ); |
There was a problem hiding this comment.
Handle missing/nullable headers before constructing Headers.
Line 14 can throw when event.headers is absent, and String(value) can emit "undefined"/"null" as actual header values. Please normalize with a safe default and drop nullish entries.
Suggested fix
- const headers = new Headers(
- Object.fromEntries(Object.entries(event.headers!).map(([key, value]) => [key, String(value)]))
- );
+ const headers = new Headers(
+ Object.fromEntries(
+ Object.entries(event.headers ?? {}).flatMap(([key, value]) =>
+ value == null ? [] : [[key, String(value)]]
+ )
+ )
+ );📝 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.
| const headers = new Headers( | |
| Object.fromEntries(Object.entries(event.headers!).map(([key, value]) => [key, String(value)])) | |
| ); | |
| const headers = new Headers( | |
| Object.fromEntries( | |
| Object.entries(event.headers ?? {}).flatMap(([key, value]) => | |
| value == null ? [] : [[key, String(value)]] | |
| ) | |
| ) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/presets/scaleway/runtime/scaleway-functions.ts` around lines 13 - 15, The
construction of the Headers object can throw when event.headers is missing and
will turn null/undefined values into the literal strings "null"/"undefined";
update the const headers creation to use a safe default and drop nullish entries
by replacing Object.entries(event.headers!) with Object.entries(event.headers ??
{}) and filter entries where value != null before mapping to strings (e.g., keep
only entries with value !== null && value !== undefined), then map values to
String(value) and pass the resulting object into new Headers so no nullish
header values are included.
| headers.get("X-Forwarded-Proto") === "http" ? "http://" : "https://", | ||
| headers.get("Host")!, | ||
| event.path |
There was a problem hiding this comment.
Replace Host non-null assertion with an explicit, actionable error.
Line 20 uses headers.get("Host")!, which can fail downstream with unclear errors. Validate once and throw a clear message before URL construction.
Suggested fix
+ const host = headers.get("Host");
+ if (!host) {
+ throw new Error("Invalid Scaleway event: missing 'Host' header, cannot construct request URL.");
+ }
+
const url = withQuery(
joinURL(
headers.get("X-Forwarded-Proto") === "http" ? "http://" : "https://",
- headers.get("Host")!,
+ host,
event.path
),
event.queryStringParameters ?? {}
);As per coding guidelines, "Prefer explicit errors over silent failures; use warnings for recoverable situations and throw for invalid states" and "Include actionable context in error messages".
📝 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.
| headers.get("X-Forwarded-Proto") === "http" ? "http://" : "https://", | |
| headers.get("Host")!, | |
| event.path | |
| const host = headers.get("Host"); | |
| if (!host) { | |
| throw new Error("Invalid Scaleway event: missing 'Host' header, cannot construct request URL."); | |
| } | |
| const url = withQuery( | |
| joinURL( | |
| headers.get("X-Forwarded-Proto") === "http" ? "http://" : "https://", | |
| host, | |
| event.path | |
| ), | |
| event.queryStringParameters ?? {} | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/presets/scaleway/runtime/scaleway-functions.ts` around lines 19 - 21, The
code blindly uses headers.get("Host")! when building the request URL (the
expression near headers.get("X-Forwarded-Proto") === "http" ...
headers.get("Host")! ... event.path), which can produce opaque downstream
failures; instead, read headers.get("Host") into a local (e.g., host) and if
it's null or empty throw a clear, actionable Error (for example "Missing Host
header in incoming request; cannot construct target URL") before continuing to
construct the URL—update the URL construction site (where headers.get("Host")!
is used) to use that validated host variable.
| event.queryStringParameters ?? {} | ||
| ); | ||
|
|
||
| const body = event.isBase64Encoded ? Buffer.from(event.body, "base64") : event.body; |
There was a problem hiding this comment.
Guard base64 decoding with a body-type check.
Line 26 assumes event.body is decodable when event.isBase64Encoded is true; invalid/missing body will throw a low-context error. Add an explicit guard first.
Suggested fix
+ if (event.isBase64Encoded && typeof event.body !== "string") {
+ throw new Error("Invalid Scaleway event: 'isBase64Encoded' requires a string 'body'.");
+ }
const body = event.isBase64Encoded ? Buffer.from(event.body, "base64") : event.body;As per coding guidelines, "Prefer explicit errors over silent failures; use warnings for recoverable situations and throw for invalid states" and "Include actionable context in error messages".
📝 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.
| const body = event.isBase64Encoded ? Buffer.from(event.body, "base64") : event.body; | |
| if (event.isBase64Encoded && typeof event.body !== "string") { | |
| throw new Error("Invalid Scaleway event: 'isBase64Encoded' requires a string 'body'."); | |
| } | |
| const body = event.isBase64Encoded ? Buffer.from(event.body, "base64") : event.body; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/presets/scaleway/runtime/scaleway-functions.ts` at line 26, The
assignment to const body using event.isBase64Encoded assumes event.body is
present and a decodable string; guard this by first checking
event.isBase64Encoded and that event.body is a non-empty string (or Buffer)
before calling Buffer.from, and if the body is missing or not a string throw a
clear Error mentioning event.isBase64Encoded and the missing/invalid event.body;
otherwise perform the base64 decode, e.g. only call Buffer.from(event.body,
"base64") when the guard passes, and keep the existing fallback to event.body
when not base64-encoded.
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/scaleway/preset.ts`:
- Line 5: The entry path in the preset object currently points to
"./scaleway/runtime/scaleway-serverless" but the actual runtime file is named
"scaleway-functions.ts"; update the entry property (the `entry` field in the
preset export in src/presets/scaleway/preset.ts) to reference
"./scaleway/runtime/scaleway-functions" so the bundler can locate the correct
runtime module.
🪄 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: 7587050c-8ac4-4ed8-8175-aef1075ad32d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
docs/2.deploy/20.providers/scaleway.mdpackage.jsonsrc/presets/_all.gen.tssrc/presets/_types.gen.tssrc/presets/scaleway/preset.tssrc/presets/scaleway/runtime/scaleway-functions.tstest/presets/scaleway.test.ts
✅ Files skipped from review due to trivial changes (4)
- package.json
- docs/2.deploy/20.providers/scaleway.md
- src/presets/_all.gen.ts
- src/presets/_types.gen.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/presets/scaleway/runtime/scaleway-functions.ts
- test/presets/scaleway.test.ts
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/presets/scaleway/runtime/scaleway-functions.ts (3)
25-25:⚠️ Potential issue | 🟡 MinorAdd a type guard before base64 decoding the body.
Line 25 assumes
event.bodyis a string whenisBase64Encodedis true. If not, decoding throws a low-context runtime error.Suggested fix
+ if (event.isBase64Encoded && typeof event.body !== "string") { + throw new Error("Invalid Scaleway event: 'isBase64Encoded' requires a string 'body'."); + } const body = event.isBase64Encoded ? Buffer.from(event.body, "base64") : event.body;As per coding guidelines, "Prefer explicit errors over silent failures; use warnings for recoverable situations and throw for invalid states" and "Include actionable context in error messages".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/scaleway/runtime/scaleway-functions.ts` at line 25, The code assumes event.body is a string when event.isBase64Encoded is true; add a type guard that checks typeof event.body === "string" before calling Buffer.from(..., "base64") and throw a clear, actionable error (e.g., "Invalid event.body: expected base64 string when isBase64Encoded is true") if the check fails; otherwise perform the decode and assign to body (keeping the existing ternary behavior around event.isBase64Encoded and body).
12-14:⚠️ Potential issue | 🟠 MajorGuard nullable headers before constructing
Headers.Line 13 can throw when
event.headersis absent, and nullish header values become literal"null"/"undefined"strings.Suggested fix
- const headers = new Headers( - Object.fromEntries(Object.entries(event.headers!).map(([key, value]) => [key, String(value)])) - ); + const headers = new Headers( + Object.fromEntries( + Object.entries(event.headers ?? {}).flatMap(([key, value]) => + value == null ? [] : [[key, String(value)]] + ) + ) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/scaleway/runtime/scaleway-functions.ts` around lines 12 - 14, The code constructs Headers directly from event.headers which can be null/undefined and will stringify nullish values to "null"/"undefined"; update the creation in scaleway-functions.ts so you first guard event.headers (e.g., const raw = event.headers ?? {}), then build entries filtering out values that are null or undefined (e.g., Object.entries(raw).filter(([,v]) => v != null).map(([k,v]) => [k, String(v)])) and pass that into new Headers(...) so the headers variable and event.headers are protected and nullish values are not turned into literal strings.
16-21:⚠️ Potential issue | 🟠 MajorValidate
Hostonce and throw an actionable error.Line 19 uses a non-null assertion on
Host, which can fail with an opaque downstream error. Validate early and fail clearly.Suggested fix
+ const host = headers.get("Host"); + if (!host) { + throw new Error("Invalid Scaleway event: missing 'Host' header, cannot construct request URL."); + } + const url = withQuery( joinURL( headers.get("X-Forwarded-Proto") === "http" ? "http://" : "https://", - headers.get("Host")!, + host, event.path ), event.queryStringParameters ?? {} );As per coding guidelines, "Prefer explicit errors over silent failures; use warnings for recoverable situations and throw for invalid states" and "Include actionable context in error messages".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/scaleway/runtime/scaleway-functions.ts` around lines 16 - 21, The code uses headers.get("Host") with a non-null assertion when building url (via joinURL/withQuery), which can cause opaque failures; retrieve and validate the Host header once (e.g., const host = headers.get("Host")), throw a clear, actionable Error if host is missing or invalid, and then pass that validated host into joinURL instead of using headers.get("Host")! so the failure is explicit and contains context about the missing Host header and how to fix it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/presets/scaleway/runtime/scaleway-functions.ts`:
- Line 25: The code assumes event.body is a string when event.isBase64Encoded is
true; add a type guard that checks typeof event.body === "string" before calling
Buffer.from(..., "base64") and throw a clear, actionable error (e.g., "Invalid
event.body: expected base64 string when isBase64Encoded is true") if the check
fails; otherwise perform the decode and assign to body (keeping the existing
ternary behavior around event.isBase64Encoded and body).
- Around line 12-14: The code constructs Headers directly from event.headers
which can be null/undefined and will stringify nullish values to
"null"/"undefined"; update the creation in scaleway-functions.ts so you first
guard event.headers (e.g., const raw = event.headers ?? {}), then build entries
filtering out values that are null or undefined (e.g.,
Object.entries(raw).filter(([,v]) => v != null).map(([k,v]) => [k, String(v)]))
and pass that into new Headers(...) so the headers variable and event.headers
are protected and nullish values are not turned into literal strings.
- Around line 16-21: The code uses headers.get("Host") with a non-null assertion
when building url (via joinURL/withQuery), which can cause opaque failures;
retrieve and validate the Host header once (e.g., const host =
headers.get("Host")), throw a clear, actionable Error if host is missing or
invalid, and then pass that validated host into joinURL instead of using
headers.get("Host")! so the failure is explicit and contains context about the
missing Host header and how to fix it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fe07d14-20ca-4390-9b05-87dc61cfbe10
📒 Files selected for processing (1)
src/presets/scaleway/runtime/scaleway-functions.ts
2 participants
🔗 Linked issue
#2973
❓ Type of change
📚 Description
Add Scaleway serverless functions preset.
📝 Checklist
Other
Open to all feedback, I'm figuring out what to add further to the documentation and preset config so deploys go as easy as possible.