feat: support after() (#2717)

pieh · ascorbic · web-flow · commit 51e5373e4914 · 2024-12-09T15:35:43.000+01:00
* test: upgrade deno used in tests

* test: e2e simple next/after test

* feat: support waitUntil

* use already existing trackBackgroundWork helper from request context to handle next/after

* test: increase timeout for one of smoke tests due to team-wide extensions installation time making it timeout

* test: move test to dedicated fixture

* chore: update outdated comment

* chore: clarify awaiting backgroundWorkPromise

* test: update assertion for next &gt;=15.0.4-canary.18

* fix: support changed shape of getRequestHandlers return

---------

Co-authored-by: Matt Kane &lt;matt.kane@netlify.com&gt;
diff --git a/.github/workflows/pre-release.yml b/.github/workflows/pre-release.yml
@@ -23,8 +23,8 @@ jobs:
       - name: Install Deno
         uses: denoland/setup-deno@v1
         with:
-          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/e55f825bd985d3c92e21d1b765d71e70d5628fba/node/bridge.ts#L17
-          deno-version: v1.37.0
+          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
+          deno-version: v1.44.4
       - name: Extract tag and version
         id: extract
         run: |-
diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml
@@ -31,8 +31,8 @@ jobs:
       - name: Install Deno
         uses: denoland/setup-deno@v1
         with:
-          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/e55f825bd985d3c92e21d1b765d71e70d5628fba/node/bridge.ts#L17
-          deno-version: v1.37.0
+          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
+          deno-version: v1.44.4
       - name: Build
         run: npm run build
         if: ${{ steps.release.outputs.release_created }}
diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml
@@ -65,8 +65,8 @@ jobs:
       - name: Install Deno
         uses: denoland/setup-deno@v1
         with:
-          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
-          deno-version: v1.37.0
+          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
+          deno-version: v1.44.4
       - name: 'Install dependencies'
         run: npm ci
       - name: 'Prepare Netlify CLI'
@@ -134,7 +134,7 @@ jobs:
         uses: denoland/setup-deno@v1
         with:
           # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/e55f825bd985d3c92e21d1b765d71e70d5628fba/node/bridge.ts#L17
-          deno-version: v1.37.0
+          deno-version: v1.44.4
       - name: 'Install dependencies'
         run: npm ci
       - name: 'Build'
@@ -198,8 +198,8 @@ jobs:
       - name: Install Deno
         uses: denoland/setup-deno@v1
         with:
-          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
-          deno-version: v1.37.0
+          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
+          deno-version: v1.44.4
       - name: 'Install dependencies'
         run: npm ci
       - name: 'Build'
diff --git a/.github/workflows/size-check.yml b/.github/workflows/size-check.yml
@@ -23,8 +23,8 @@ jobs:
       - name: Install Deno
         uses: denoland/setup-deno@v1
         with:
-          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
-          deno-version: v1.37.0
+          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
+          deno-version: v1.44.4
       - run: npm ci
 
       - name: Package size report
diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml
@@ -161,8 +161,8 @@ jobs:
       - name: Install Deno
         uses: denoland/setup-deno@v1
         with:
-          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
-          deno-version: v1.37.0
+          # Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
+          deno-version: v1.44.4
 
       - name: install runtime
         run: npm install --ignore-scripts
diff --git a/src/build/templates/handler-monorepo.tmpl.js b/src/build/templates/handler-monorepo.tmpl.js
@@ -16,7 +16,7 @@ export default async function (req, context) {
     tracing.start()
   }
 
-  const requestContext = createRequestContext(req)
+  const requestContext = createRequestContext(req, context)
   const tracer = getTracer()
 
   const handlerResponse = await runWithRequestContext(requestContext, () => {
diff --git a/src/build/templates/handler.tmpl.js b/src/build/templates/handler.tmpl.js
@@ -13,7 +13,7 @@ export default async function handler(req, context) {
   if (process.env.NETLIFY_OTLP_TRACE_EXPORTER_URL) {
     tracing.start()
   }
-  const requestContext = createRequestContext(req)
+  const requestContext = createRequestContext(req, context)
   const tracer = getTracer()
 
   const handlerResponse = await runWithRequestContext(requestContext, () => {
diff --git a/src/run/handlers/request-context.cts b/src/run/handlers/request-context.cts
@@ -1,11 +1,17 @@
 import { AsyncLocalStorage } from 'node:async_hooks'
 
+import type { Context } from '@netlify/functions'
 import { LogLevel, systemLogger } from '@netlify/functions/internal'
 
 import type { NetlifyCachedRouteValue } from '../../shared/cache-types.cjs'
 
 type SystemLogger = typeof systemLogger
 
+// TODO: remove once public types are updated
+export interface FutureContext extends Context {
+  waitUntil?: (promise: Promise<unknown>) => void
+}
+
 export type RequestContext = {
   captureServerTiming: boolean
   responseCacheGetLastModified?: number
@@ -16,25 +22,33 @@ export type RequestContext = {
   serverTiming?: string
   routeHandlerRevalidate?: NetlifyCachedRouteValue['revalidate']
   /**
-   * Track promise running in the background and need to be waited for
+   * Track promise running in the background and need to be waited for.
+   * Uses `context.waitUntil` if available, otherwise stores promises to
+   * await on.
    */
   trackBackgroundWork: (promise: Promise<unknown>) => void
   /**
-   * Promise that need to be executed even if response was already sent
+   * Promise that need to be executed even if response was already sent.
+   * If `context.waitUntil` is available this promise will be always resolved
+   * because background work tracking was offloaded to `context.waitUntil`.
    */
   backgroundWorkPromise: Promise<unknown>
   logger: SystemLogger
 }
 
 type RequestContextAsyncLocalStorage = AsyncLocalStorage<RequestContext>
 
-export function createRequestContext(request?: Request): RequestContext {
+export function createRequestContext(request?: Request, context?: FutureContext): RequestContext {
   const backgroundWorkPromises: Promise<unknown>[] = []
 
   return {
     captureServerTiming: request?.headers.has('x-next-debug-logging') ?? false,
     trackBackgroundWork: (promise) => {
-      backgroundWorkPromises.push(promise)
+      if (context?.waitUntil) {
+        context.waitUntil(promise)
+      } else {
+        backgroundWorkPromises.push(promise)
+      }
     },
     get backgroundWorkPromise() {
       return Promise.allSettled(backgroundWorkPromises)
diff --git a/src/run/handlers/server.ts b/src/run/handlers/server.ts
@@ -1,7 +1,6 @@
 import type { OutgoingHttpHeaders } from 'http'
 
 import { ComputeJsOutgoingMessage, toComputeResponse, toReqRes } from '@fastly/http-compute-js'
-import { Context } from '@netlify/functions'
 import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
 import type { WorkerRequestHandler } from 'next/dist/server/lib/types.js'
 
@@ -16,9 +15,12 @@ import { nextResponseProxy } from '../revalidate.js'
 
 import { createRequestContext, getLogger, getRequestContext } from './request-context.cjs'
 import { getTracer } from './tracer.cjs'
+import { setupWaitUntil } from './wait-until.cjs'
 
 const nextImportPromise = import('../next.cjs')
 
+setupWaitUntil()
+
 let nextHandler: WorkerRequestHandler, nextConfig: NextConfigComplete
 
 /**
@@ -44,13 +46,7 @@ const disableFaultyTransferEncodingHandling = (res: ComputeJsOutgoingMessage) =>
   }
 }
 
-// TODO: remove once https://github.com/netlify/serverless-functions-api/pull/219
-// is released and public types are updated
-interface FutureContext extends Context {
-  waitUntil?: (promise: Promise<unknown>) => void
-}
-
-export default async (request: Request, context: FutureContext) => {
+export default async (request: Request) => {
   const tracer = getTracer()
 
   if (!nextHandler) {
@@ -60,10 +56,10 @@ export default async (request: Request, context: FutureContext) => {
       nextConfig = await getRunConfig()
       setRunConfig(nextConfig)
 
-      const { getMockedRequestHandlers } = await nextImportPromise
+      const { getMockedRequestHandler } = await nextImportPromise
       const url = new URL(request.url)
 
-      ;[nextHandler] = await getMockedRequestHandlers({
+      nextHandler = await getMockedRequestHandler({
         port: Number(url.port) || 443,
         hostname: url.hostname,
         dir: process.cwd(),
@@ -128,19 +124,20 @@ export default async (request: Request, context: FutureContext) => {
       return new Response(body || null, response)
     }
 
-    if (context.waitUntil) {
-      context.waitUntil(requestContext.backgroundWorkPromise)
-    }
-
     const keepOpenUntilNextFullyRendered = new TransformStream({
       async flush() {
         // it's important to keep the stream open until the next handler has finished
         await nextHandlerPromise
-        if (!context.waitUntil) {
-          // if waitUntil is not available, we have to keep response stream open until background promises are resolved
-          // to ensure that all background work executes
-          await requestContext.backgroundWorkPromise
-        }
+
+        // Next.js relies on `close` event emitted by response to trigger running callback variant of `next/after`
+        // however @fastly/http-compute-js never actually emits that event - so we have to emit it ourselves,
+        // otherwise Next would never run the callback variant of `next/after`
+        res.emit('close')
+
+        // We have to keep response stream open until tracked background promises that are don't use `context.waitUntil`
+        // are resolved. If `context.waitUntil` is available, `requestContext.backgroundWorkPromise` will be empty
+        // resolved promised and so awaiting it is no-op
+        await requestContext.backgroundWorkPromise
       },
     })
 
diff --git a/src/run/handlers/wait-until.cts b/src/run/handlers/wait-until.cts
@@ -0,0 +1,26 @@
+import { getRequestContext } from './request-context.cjs'
+
+/**
+ *  @see https://github.com/vercel/next.js/blob/canary/packages/next/src/server/after/builtin-request-context.ts
+ */
+const NEXT_REQUEST_CONTEXT_SYMBOL = Symbol.for('@next/request-context')
+
+export type NextJsRequestContext = {
+  get(): { waitUntil?: (promise: Promise<unknown>) => void } | undefined
+}
+
+type GlobalThisWithRequestContext = typeof globalThis & {
+  [NEXT_REQUEST_CONTEXT_SYMBOL]?: NextJsRequestContext
+}
+
+/**
+ * Registers a `waitUntil` to be used by Next.js for next/after
+ */
+export function setupWaitUntil() {
+  // eslint-disable-next-line @typescript-eslint/no-extra-semi
+  ;(globalThis as GlobalThisWithRequestContext)[NEXT_REQUEST_CONTEXT_SYMBOL] = {
+    get() {
+      return { waitUntil: getRequestContext()?.trackBackgroundWork }
+    },
+  }
+}
diff --git a/src/run/next.cts b/src/run/next.cts
@@ -85,7 +85,7 @@ export type HtmlBlob = {
   isFallback: boolean
 }
 
-export async function getMockedRequestHandlers(...args: Parameters<typeof getRequestHandlers>) {
+export async function getMockedRequestHandler(...args: Parameters<typeof getRequestHandlers>) {
   const tracer = getTracer()
   return tracer.withActiveSpan('mocked request handler', async () => {
     const ofs = { ...fs }
@@ -131,6 +131,9 @@ export async function getMockedRequestHandlers(...args: Parameters<typeof getReq
       require('fs').promises,
     )
 
-    return getRequestHandlers(...args)
+    const requestHandlers = await getRequestHandlers(...args)
+    // depending on Next.js version requestHandlers might be an array of object
+    // see https://github.com/vercel/next.js/commit/08e7410f15706379994b54c3195d674909a8d533#diff-37243d614f1f5d3f7ea50bbf2af263f6b1a9a4f70e84427977781e07b02f57f1R742
+    return Array.isArray(requestHandlers) ? requestHandlers[0] : requestHandlers.requestHandler
   })
 }
diff --git a/tests/e2e/after.test.ts b/tests/e2e/after.test.ts
@@ -0,0 +1,42 @@
+import { expect } from '@playwright/test'
+import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs'
+import { test } from '../utils/playwright-helpers.js'
+
+test('next/after callback is executed and finishes', async ({ page, after }) => {
+  test.skip(!nextVersionSatisfies('>=15.0.0'), 'This test is only for Next.js 15+')
+
+  // trigger initial request to check page which might be stale and allow regenerating in background
+  await page.goto(`${after.url}/after/check`)
+
+  await new Promise((resolve) => setTimeout(resolve, 5000))
+
+  // after it was possibly regenerated we can start checking actual content of the page
+  await page.goto(`${after.url}/after/check`)
+  const pageInfoLocator1 = await page.locator('#page-info')
+  const pageInfo1 = JSON.parse((await pageInfoLocator1.textContent()) ?? '{}')
+
+  expect(typeof pageInfo1?.timestamp, 'Check page should have timestamp').toBe('number')
+
+  await page.goto(`${after.url}/after/check`)
+  const pageInfoLocator2 = await page.locator('#page-info')
+  const pageInfo2 = JSON.parse((await pageInfoLocator2.textContent()) ?? '{}')
+
+  expect(typeof pageInfo2?.timestamp, 'Check page should have timestamp').toBe('number')
+
+  expect(pageInfo2.timestamp, 'Check page should be cached').toBe(pageInfo1.timestamp)
+
+  await page.goto(`${after.url}/after/trigger`)
+
+  // wait for next/after to trigger revalidation of check page
+  await new Promise((resolve) => setTimeout(resolve, 5000))
+
+  await page.goto(`${after.url}/after/check`)
+  const pageInfoLocator3 = await page.locator('#page-info')
+  const pageInfo3 = JSON.parse((await pageInfoLocator3.textContent()) ?? '{}')
+
+  expect(typeof pageInfo3?.timestamp, 'Check page should have timestamp').toBe('number')
+  expect(
+    pageInfo3.timestamp,
+    'Check page should be invalidated with newer timestamp',
+  ).toBeGreaterThan(pageInfo1.timestamp)
+})
diff --git a/tests/fixtures/after/app/after/check/page.js b/tests/fixtures/after/app/after/check/page.js
@@ -0,0 +1,10 @@
+export const revalidate = 3600 // arbitrarily long, just so that it doesn't happen during a test run
+
+export default async function Page() {
+  const data = {
+    timestamp: Date.now(),
+  }
+  console.log('/timestamp/key/[key] rendered', data)
+
+  return <div id="page-info">{JSON.stringify(data)}</div>
+}
diff --git a/tests/fixtures/after/app/after/trigger/page.js b/tests/fixtures/after/app/after/trigger/page.js
@@ -0,0 +1,17 @@
+import { revalidatePath } from 'next/cache'
+import { unstable_after as after, connection } from 'next/server'
+
+export default async function Page() {
+  await connection()
+  after(async () => {
+    // this will run after response was sent
+    console.log('after() triggered')
+    console.log('after() sleep 1s')
+    await new Promise((resolve) => setTimeout(resolve, 1000))
+
+    console.log('after() revalidatePath /after/check')
+    revalidatePath('/after/check')
+  })
+
+  return <div>Page with after()</div>
+}
diff --git a/tests/fixtures/after/app/layout.js b/tests/fixtures/after/app/layout.js
@@ -0,0 +1,12 @@
+export const metadata = {
+  title: 'Simple Next App',
+  description: 'Description for Simple Next App',
+}
+
+export default function RootLayout({ children }) {
+  return (
+    <html lang="en">
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/tests/fixtures/after/next.config.js b/tests/fixtures/after/next.config.js
@@ -0,0 +1,12 @@
+/** @type {import('next').NextConfig} */
+const nextConfig = {
+  output: 'standalone',
+  eslint: {
+    ignoreDuringBuilds: true,
+  },
+  experimental: {
+    after: true,
+  },
+}
+
+module.exports = nextConfig
diff --git a/tests/fixtures/after/package.json b/tests/fixtures/after/package.json
@@ -0,0 +1,20 @@
+{
+  "name": "after",
+  "version": "0.1.0",
+  "private": true,
+  "scripts": {
+    "postinstall": "next build",
+    "dev": "next dev",
+    "build": "next build"
+  },
+  "dependencies": {
+    "next": "latest",
+    "react": "18.2.0",
+    "react-dom": "18.2.0"
+  },
+  "test": {
+    "dependencies": {
+      "next": ">=15.0.0"
+    }
+  }
+}
diff --git a/tests/integration/simple-app.test.ts b/tests/integration/simple-app.test.ts
diff --git a/tests/smoke/deploy.test.ts b/tests/smoke/deploy.test.ts
diff --git a/tests/utils/create-e2e-fixture.ts b/tests/utils/create-e2e-fixture.ts

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ export default async function (req, context) {`
`16`	`16`	`tracing.start()`
`17`	`17`	`}`
`18`	`18`
`19`		`- const requestContext = createRequestContext(req)`
	`19`	`+ const requestContext = createRequestContext(req, context)`
`20`	`20`	`const tracer = getTracer()`
`21`	`21`
`22`	`22`	`const handlerResponse = await runWithRequestContext(requestContext, () => {`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ export default async function handler(req, context) {`
`13`	`13`	`if (process.env.NETLIFY_OTLP_TRACE_EXPORTER_URL) {`
`14`	`14`	`tracing.start()`
`15`	`15`	`}`
`16`		`- const requestContext = createRequestContext(req)`
	`16`	`+ const requestContext = createRequestContext(req, context)`
`17`	`17`	`const tracer = getTracer()`
`18`	`18`
`19`	`19`	`const handlerResponse = await runWithRequestContext(requestContext, () => {`