From 4fd8bc0be500eff4c7348b716449224294e512e2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eduardo=20Bou=C3=A7as?= <mail@eduardoboucas.com>
Date: Fri, 15 Mar 2024 11:14:35 +0000
Subject: [PATCH] fix: improve handling of scheduled v2 functions

---
 src/lib/functions/server.ts                     | 17 ++++++++++++++++-
 .../functions/scheduled-v2.mjs                  |  7 +++++++
 .../commands/dev/scheduled-functions.test.ts    |  2 +-
 3 files changed, 24 insertions(+), 2 deletions(-)
 create mode 100644 tests/integration/__fixtures__/dev-server-with-functions/functions/scheduled-v2.mjs

diff --git a/src/lib/functions/server.ts b/src/lib/functions/server.ts
index 7b07d049238..78c9feb1cff 100644
--- a/src/lib/functions/server.ts
+++ b/src/lib/functions/server.ts
@@ -201,6 +201,12 @@ export const createHandler = function (options: GetFunctionsServerOptions): Requ
 
       handleBackgroundFunctionResult(functionName, error)
     } else if (await func.isScheduled()) {
+      // In production, scheduled functions always receive POST requests, so we
+      // have to emulate that here, even if a user has triggered a GET request
+      // as part of their tests. If we don't do this, we'll hit problems when
+      // we send the invocation body in a request that can't have a body.
+      event.httpMethod = 'POST'
+
       const { error, result } = await func.invoke(
         {
           ...event,
@@ -219,9 +225,18 @@ export const createHandler = function (options: GetFunctionsServerOptions): Requ
 
       handleScheduledFunction({
         error,
-        result,
         request,
         response,
+
+        // When we handle the result of invoking a scheduled function, we'll warn
+        // people in case their function returned a body or headers, since those
+        // will have no practical effect in production. However, in v2 functions
+        // we don't currently have a good way of asserting whether the body we're
+        // seeing has been actually produced by user code or by the bootstrap, so
+        // we risk printing that warn unnecessarily, which causes more harm than
+        // good. Until we find a way of making this detection better, ignore the
+        // invocation result entirely for v2 functions.
+        result: func.runtimeAPIVersion === 1 ? result : {},
       })
     } else {
       const { error, result } = await func.invoke(event, clientContext)
diff --git a/tests/integration/__fixtures__/dev-server-with-functions/functions/scheduled-v2.mjs b/tests/integration/__fixtures__/dev-server-with-functions/functions/scheduled-v2.mjs
new file mode 100644
index 00000000000..c9315d13d30
--- /dev/null
+++ b/tests/integration/__fixtures__/dev-server-with-functions/functions/scheduled-v2.mjs
@@ -0,0 +1,7 @@
+export default async () => {
+  console.log('Hello!')
+}
+
+export const config = {
+  schedule: '* * * * *',
+}
diff --git a/tests/integration/commands/dev/scheduled-functions.test.ts b/tests/integration/commands/dev/scheduled-functions.test.ts
index 27f62494e0e..ea6dedbdd09 100644
--- a/tests/integration/commands/dev/scheduled-functions.test.ts
+++ b/tests/integration/commands/dev/scheduled-functions.test.ts
@@ -7,7 +7,7 @@ import { pause } from '../../utils/pause.js'
 describe('scheduled functions', () => {
   setupFixtureTests('dev-server-with-functions', { devServer: true }, () => {
     test<FixtureTestContext>('should emulate next_run for scheduled functions', async ({ devServer }) => {
-      const response = await fetch(`http://localhost:${devServer.port}/.netlify/functions/scheduled-isc`, {})
+      const response = await fetch(`http://localhost:${devServer.port}/.netlify/functions/scheduled-v2`, {})
 
       expect(response.status).toBe(200)
     })