Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions blocks/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export interface RequestState {
};
bag: WeakMap<any, any>;
flags: Flag[];
dirty?: boolean;
dirtyTraces?: string[];
}

export type FnContext<
Expand Down
110 changes: 53 additions & 57 deletions runtime/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ export const proxyState = (
};

export type DecoMiddleware<TManifest extends AppManifest = AppManifest> =
MiddlewareHandler<
DecoRouteState<TManifest>
>;
MiddlewareHandler<DecoRouteState<TManifest>>;

export type DecoRouteState<TManifest extends AppManifest = AppManifest> = {
Variables: State<TManifest>;
Expand Down Expand Up @@ -107,10 +105,11 @@ async (ctx, next) => {

const DEBUG_COOKIE = "deco_debug";
const DEBUG_ENABLED = "enabled";
const PAGE_CACHE_DRY_RUN = Deno.env.get("DECO_PAGE_CACHE_DRY_RUN") === "true";

export const DEBUG_QS = "__d";
const addHours = function (date: Date, h: number) {
date.setTime(date.getTime() + (h * 60 * 60 * 1000));
const addHours = (date: Date, h: number) => {
date.setTime(date.getTime() + h * 60 * 60 * 1000);
return date;
};

Expand All @@ -136,20 +135,19 @@ export const DEBUG = {
): { action: DebugAction; enabled: boolean; correlationId: string } => {
const url = new URL(request.url);
const debugFromCookies = getCookies(request.headers)[DEBUG_COOKIE];
const debugFromQS = url.searchParams.has(DEBUG_QS) && DEBUG_ENABLED ||
const debugFromQS = (url.searchParams.has(DEBUG_QS) && DEBUG_ENABLED) ||
url.searchParams.get(DEBUG_COOKIE);
const hasDebugFromQS = debugFromQS !== null;
const isLivePreview = url.pathname.includes("/live/previews/");
const enabled = ((debugFromQS ?? debugFromCookies) === DEBUG_ENABLED) ||
const enabled = (debugFromQS ?? debugFromCookies) === DEBUG_ENABLED ||
isLivePreview;

const correlationId = url.searchParams.get(DEBUG_QS) ||
crypto.randomUUID();
const correlationId = url.searchParams.get(DEBUG_QS) || crypto.randomUUID();
const liveContext = Context.active();
// querystring forces a setcookie using the querystring value
return {
action: hasDebugFromQS || isLivePreview
? (enabled
? enabled
? async (resp) => {
DEBUG.enable(resp);
resp.headers.set("x-correlation-id", correlationId);
Expand All @@ -159,7 +157,7 @@ export const DEBUG = {
);
resp.headers.set(
"x-deco-revision",
`${await liveContext.release?.revision() ?? ""}`,
`${(await liveContext.release?.revision()) ?? ""}`,
);
resp.headers.set(
"x-isolate-started-at",
Expand All @@ -171,7 +169,7 @@ export const DEBUG = {
`${liveContext.instance.readyAt.toISOString()}`,
);
}
: DEBUG.disable)
: DEBUG.disable
: DEBUG.none,
enabled,
correlationId,
Expand Down Expand Up @@ -221,19 +219,14 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
liveness,
// 1 => statebuilder
async (ctx, next) => {
const { enabled, action, correlationId } = DEBUG.fromRequest(
ctx.req.raw,
);
const { enabled, action, correlationId } = DEBUG.fromRequest(ctx.req.raw);

//@ts-ignore: ctx.base dont exist in hono ctx
//@ts-expect-error: ctx.base dont exist in hono ctx
ctx.base = ctx.var.global;
const state = await deco.prepareState(
ctx,
{
enabled,
correlationId,
},
);
const state = await deco.prepareState(ctx, {
enabled,
correlationId,
});
for (const [key, value] of Object.entries(state)) {
ctx.set(key as keyof typeof state, value);
}
Expand All @@ -253,7 +246,7 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
if (ctx.req.raw.headers.get("upgrade") === "websocket") {
return;
}
ctx.res && await action(ctx.res);
ctx.res && (await action(ctx.res));
setLogger(null);
},
// 2 => observability
Expand All @@ -268,8 +261,7 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
"http.request.url": ctx.req.raw.url,
"http.request.method": ctx.req.raw.method,
"http.request.body.size":
ctx.req.raw.headers.get("content-length") ??
undefined,
ctx.req.raw.headers.get("content-length") ?? undefined,
"url.scheme": url.protocol,
"server.address": url.host,
"url.query": url.search,
Expand All @@ -296,10 +288,7 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
span.setStatus({
code: isErr ? SpanStatusCode.ERROR : SpanStatusCode.OK,
});
span.setAttribute(
"http.response.status_code",
`${status}`,
);
span.setAttribute("http.response.status_code", `${status}`);
if (ctx?.var?.pathTemplate) {
const route = `${ctx.req.raw.method} ${ctx?.var?.pathTemplate}`;
span.updateName(route);
Expand All @@ -310,9 +299,7 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
ctx.res?.status ?? 500,
);
} else {
span.updateName(
`${ctx.req.raw.method} ${ctx.req.raw.url}`,
);
span.updateName(`${ctx.req.raw.method} ${ctx.req.raw.url}`);
}
span.end();
if (!url.pathname.startsWith("/_frsh")) {
Expand All @@ -333,10 +320,8 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
},
// 3 => main
async (ctx, next) => {
if (
ctx.req.raw.method === "HEAD" && isMonitoringRobots(ctx.req.raw)
) {
return ctx.res = new Response(null, { status: 200 });
if (ctx.req.raw.method === "HEAD" && isMonitoringRobots(ctx.req.raw)) {
return (ctx.res = new Response(null, { status: 200 }));
}

const url = new URL(ctx.req.raw.url); // TODO(mcandeia) check if ctx.url can be used here
Expand All @@ -354,7 +339,7 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
// which means that sometimes it will fail as headers are immutable.
// so I'm first setting it to undefined and just then set the entire response again
ctx.res = undefined;
return ctx.res = initialResponse;
return (ctx.res = initialResponse);
}
const newHeaders = new Headers(initialResponse.headers);
context.platform && newHeaders.set("x-deco-platform", context.platform);
Expand All @@ -366,11 +351,9 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
url.pathname.startsWith("/_frsh/") ||
shouldAllowCorsForOptions
) {
Object.entries(allowCorsFor(ctx.req.raw)).map(
([name, value]) => {
newHeaders.set(name, value);
},
);
Object.entries(allowCorsFor(ctx.req.raw)).map(([name, value]) => {
newHeaders.set(name, value);
});
Comment on lines +354 to +356
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use forEach instead of map for side-effect-only iteration.

map() builds and immediately discards an array of undefined values. Biome correctly flags this as a suspicious pattern.

♻️ Proposed fix
-      Object.entries(allowCorsFor(ctx.req.raw)).map(([name, value]) => {
-        newHeaders.set(name, value);
-      });
+      for (const [name, value] of Object.entries(allowCorsFor(ctx.req.raw))) {
+        newHeaders.set(name, 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.

Suggested change
Object.entries(allowCorsFor(ctx.req.raw)).map(([name, value]) => {
newHeaders.set(name, value);
});
for (const [name, value] of Object.entries(allowCorsFor(ctx.req.raw))) {
newHeaders.set(name, value);
}
🧰 Tools
🪛 Biome (2.4.4)

[error] 354-354: This callback passed to map() iterable method should always return a value.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@runtime/middleware.ts` around lines 354 - 356, The code uses
Object.entries(allowCorsFor(ctx.req.raw)).map(...) purely for side-effects which
creates a discarded array; replace the map call with forEach to avoid allocating
that array: iterate over
Object.entries(allowCorsFor(ctx.req.raw)).forEach(([name, value]) => {
newHeaders.set(name, value); }) so change the invocation on the result of
Object.entries and keep the callback using newHeaders.set and the same
ctx.req.raw input.

}
ctx.var.response.headers.forEach((value, key) =>
newHeaders.append(key, value)
Expand All @@ -380,8 +363,7 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
reduceServerTimingsTo(printTimings(), SERVER_TIMING_MAX_LEN);
printedTimings && newHeaders.set("Server-Timing", printedTimings);

const responseStatus = ctx.var.response.status ??
initialResponse.status;
const responseStatus = ctx.var.response.status ?? initialResponse.status;

if (
url.pathname.startsWith("/_frsh/") &&
Expand All @@ -405,10 +387,7 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
() => decodeCookie(currentCookies[DECO_SEGMENT]),
"",
);
const segment = tryOrDefault(
() => JSON.parse(cookieSegment),
{},
);
const segment = tryOrDefault(() => JSON.parse(cookieSegment), {});

const active = new Set(segment.active || []);
const inactiveDrawn = new Set(segment.inactiveDrawn || []);
Expand All @@ -432,20 +411,37 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(

if (hasFlags && cookieSegment !== value) {
const date = new Date();
date.setTime(date.getTime() + (30 * 24 * 60 * 60 * 1000)); // 1 month
setCookie(newHeaders, {
name: DECO_SEGMENT,
value,
path: "/",
expires: date,
sameSite: "Lax",
}, { encode: true });
date.setTime(date.getTime() + 30 * 24 * 60 * 60 * 1000); // 1 month
setCookie(
newHeaders,
{
name: DECO_SEGMENT,
value,
path: "/",
expires: date,
sameSite: "Lax",
},
{ encode: true },
);
}
}

// If response has set-cookie header, set cache-control to no-store
if (getSetCookies(newHeaders).length > 0) {
newHeaders.set("Cache-Control", "no-store, no-cache, must-revalidate");
} else if (!ctx.var.dirty) {
if (PAGE_CACHE_DRY_RUN) {
console.warn(`[page-cache] cacheable: ${url.pathname}`);
} else {
newHeaders.set("Cache-Control", "public, max-age=120, s-maxage=120");
}
} else if (PAGE_CACHE_DRY_RUN) {
console.warn(
`[page-cache] not cacheable (cookies accessed): ${url.pathname}`,
);
for (const trace of ctx.var.dirtyTraces ?? []) {
console.warn(`[page-cache] trace:\n${trace}`);
}
}

// for some reason hono deletes content-type when response is not fresh.
Expand Down
Loading
Loading