fix(start-server-core): return a Response to non-RPC server function callers

.changeset/server-fn-form-action-response.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/start-server-core': patch
+---
+
+Fix a server function used as a form action failing with "It looks like you forgot to return a response from your server route handler" unless its handler returned a `Response`. A native form submission does not send the RPC header, and that path returned the handler's raw value straight to the HTTP layer. Non-RPC callers now receive a serialized `Response`, while a handler-provided `Response` or `redirect()` is still passed through untouched.

docs/start/framework/react/guide/server-functions.md

@@ -396,6 +396,8 @@ Return `Response` objects binary data, or custom content types.
 
 Use server functions without JavaScript by leveraging the `.url` property with HTML forms.
 
+A native form submission is not an RPC call, so the response goes to the browser rather than to the client runtime. Return a `Response`, or a `redirect()`, to control what the browser does next. A redirect is the usual choice, because it sends the browser to a real page instead of leaving it on the server function URL. Any other serializable return value is sent as a JSON body, which the browser will display as-is.
+
 ### Middleware
 
 Compose server functions with middleware for authentication, logging, and shared logic. See the [Middleware guide](./middleware.md).

packages/start-server-core/src/server-functions-handler.ts

@@ -166,10 +166,17 @@ export const handleServerAction = async ({
       }
 
       if (!isServerFn) {
-        return unwrapped
-      }
-
-      if (unwrapped instanceof Response) {
+        // Non-RPC callers, such as a native form submission that posts to
+        // `serverFn.url` or a direct HTTP request, get the handler's own
+        // Response (including a redirect) untouched. Any other value still
+        // has to be serialized: returning it raw lets a plain JS value escape
+        // to the HTTP layer, which then fails the request with "you forgot to
+        // return a response from your server route handler" instead of
+        // delivering the result.
+        if (unwrapped instanceof Response) {
+          return unwrapped
+        }
+      } else if (unwrapped instanceof Response) {
         if (isRedirect(unwrapped)) {
           return unwrapped
         }

packages/start-server-core/tests/server-functions-handler.test.ts

@@ -0,0 +1,155 @@
+// @vitest-environment node
+import { beforeEach, describe, expect, test, vi } from 'vitest'
+
+const action = vi.fn()
+
+vi.mock('../src/getServerFnById', () => ({
+  getServerFnById: vi.fn(async () => action),
+}))
+
+const { handleServerAction } = await import('../src/server-functions-handler')
+const { requestHandler } = await import('../src/request-response')
+const { runWithStartContext } = await import('@tanstack/start-storage-context')
+
+const SERVER_FN_ID = 'test-server-fn'
+
+/**
+ * Invoke `handleServerAction` the way the request pipeline does, and hand back
+ * whatever it returned so the test can assert on the raw value. The pipeline
+ * requires a Response, so returning anything else is the defect under test.
+ */
+async function invokeServerFn(options: {
+  returns: { result?: unknown; error?: unknown }
+  /** Set the header the RPC client sends. Native form posts do not send it. */
+  rpc?: boolean
+}) {
+  action.mockImplementation(async () => options.returns)
+
+  const headers: Record<string, string> = {
+    'Content-Type': 'application/x-www-form-urlencoded',
+  }
+  if (options.rpc) {
+    headers['x-tsr-serverFn'] = 'true'
+  }
+
+  const request = new Request(`http://localhost/_serverFn/${SERVER_FN_ID}`, {
+    method: 'POST',
+    headers,
+    body: 'name=Sean',
+  })
+
+  let returned: unknown
+  const handler = requestHandler(async () =>
+    runWithStartContext(
+      {
+        getRouter: () => ({}) as any,
+        request,
+        startOptions: {},
+        contextAfterGlobalMiddlewares: {},
+        executedRequestMiddlewares: new Set(),
+        handlerType: 'serverFn',
+      },
+      async () => {
+        returned = await handleServerAction({
+          request,
+          context: {},
+          serverFnId: SERVER_FN_ID,
+        })
+        return new Response('unused')
+      },
+    ),
+  )
+
+  await handler(request, {})
+  return returned
+}
+
+beforeEach(() => {
+  action.mockReset()
+  Object.assign(action, { method: 'POST' })
+})
+
+describe('handleServerAction with a non-RPC caller', () => {
+  test('serializes a plain object into a Response', async () => {
+    const returned = await invokeServerFn({ returns: { result: { ok: true } } })
+
+    expect(returned).toBeInstanceOf(Response)
+    const response = returned as Response
+    expect(response.status).toBe(200)
+    expect(response.headers.get('Content-Type')).toBe('application/json')
+    await expect(response.text()).resolves.toContain('ok')
+  })
+
+  test('returns a Response for a handler that returns nothing', async () => {
+    const returned = await invokeServerFn({ returns: { result: undefined } })
+
+    expect(returned).toBeInstanceOf(Response)
+    expect((returned as Response).status).toBe(200)
+  })
+
+  test('returns a Response when the handler produced an error', async () => {
+    const returned = await invokeServerFn({
+      returns: { result: undefined, error: new Error('boom') },
+    })
+
+    expect(returned).toBeInstanceOf(Response)
+  })
+
+  test('passes a handler-provided Response through untouched', async () => {
+    const returned = await invokeServerFn({
+      returns: {
+        result: new Response('done', {
+          status: 201,
+          headers: { 'Content-Type': 'text/plain' },
+        }),
+      },
+    })
+
+    expect(returned).toBeInstanceOf(Response)
+    const response = returned as Response
+    expect(response.status).toBe(201)
+    expect(response.headers.get('Content-Type')).toBe('text/plain')
+    // A non-RPC caller is a browser or an external client, so the internal
+    // raw-response marker must not be added.
+    expect(response.headers.get('x-tss-raw')).toBeNull()
+    await expect(response.text()).resolves.toBe('done')
+  })
+
+  test('passes a redirect Response through untouched', async () => {
+    const { redirect } = await import('@tanstack/router-core')
+    const returned = await invokeServerFn({
+      returns: { result: redirect({ href: '/after-submit', statusCode: 302 }) },
+    })
+
+    expect(returned).toBeInstanceOf(Response)
+    const response = returned as Response
+    expect(response.status).toBe(302)
+    expect(response.headers.get('Location')).toBe('/after-submit')
+  })
+})
+
+describe('handleServerAction with the RPC client', () => {
+  test('still serializes a plain object', async () => {
+    const returned = await invokeServerFn({
+      returns: { result: { ok: true } },
+      rpc: true,
+    })
+
+    expect(returned).toBeInstanceOf(Response)
+    const response = returned as Response
+    expect(response.headers.get('Content-Type')).toBe('application/json')
+    expect(response.headers.get('x-tss-serialized')).toBe('true')
+  })
+
+  test('still marks a raw Response for the client to unwrap', async () => {
+    const returned = await invokeServerFn({
+      returns: { result: new Response('done', { status: 201 }) },
+      rpc: true,
+    })
+
+    expect(returned).toBeInstanceOf(Response)
+    const response = returned as Response
+    expect(response.status).toBe(201)
+    expect(response.headers.get('x-tss-raw')).toBe('true')
+  })
+})