vercel / next.js

The React Framework
https://nextjs.org
MIT License
127.43k stars 27.03k forks source link

Server functions silently return undefined when response Content-Type is not RSC #50659

Open Ephem opened 1 year ago

Ephem commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.3.0: Mon Jan 30 20:39:46 PST 2023; root:xnu-8792.81.3~2/RELEASE_ARM64_T6020
    Binaries:
      Node: 18.13.0
      npm: 8.19.3
      Yarn: 1.22.19
      pnpm: 7.29.1
    Relevant packages:
      next: 13.4.5-canary.3
      eslint-config-next: 13.3.4
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.0.4

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true)

Link to the code that reproduces this issue or a replay of the bug

https://codesandbox.io/p/sandbox/server-functions-intercept-undefined-i4gedd

To Reproduce

Click the button, check the console for output

The important steps:

Describe the Bug

The reason I believe this is a bug is because right now it's essentially impossible to differentiate between a server function call that succeeded but returned undefined, and one that failed because some middleware (or something else), intercepted it. The common case, and what I hit, is probably auth middleware.

I've tracked the behaviour to server-action-reducer:

Expected Behavior

Should probably reject with an error, possibly including the status code of the response which might be valuable in this case?

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

manuganji commented 1 year ago

I'm experiencing this right now. If I remove middleware, the server action works.

Laecherlich commented 1 year ago

I don't know if it is the same issue but for me server actions return undefined when I return something like return NextResponse.next({ headers: requestHeaders, request: { headers: requestHeaders, }, }); From a middleware function. Without setting the headers it works perfectly.

manuganji commented 1 year ago

Yes, this was the issue. Sorry, I had fixed it back then but forgot to return here and share the fix. I was setting the CSP headers in middleware.ts but this snippet wrongly overrode the request headers when it should be only setting response headers.

https://github.com/manuganji/indiestack/blob/b978921467bf5774324d643145e3cb7970e01010/middleware.ts#L25-L42

Diwanshumidha commented 1 year ago

I am facing similar issue and not even using middlware.tsx

nicholas-yo commented 11 months ago

guys, i don't know if this is the best approach but adding the header "next-action" to "missing" in the config has solved the problem, for me.

my middleware file looks like this:

import { NextRequest, NextResponse } from "next/server";
import { generateCSP } from "./lib/generate-csp";
import { generateNonce } from "./lib/generate-nonce";
import { directives } from "./config/directives";

export const middleware = (req: NextRequest) => {
  const nonce = generateNonce();

  const cspHeader = generateCSP(directives({ nonce }));

  const requestHeaders = new Headers(req.headers);

  requestHeaders.set("x-nonce", nonce);

  requestHeaders.set("content-security-policy", cspHeader);

  return NextResponse.next({
    headers: requestHeaders,
    request: {
      headers: requestHeaders,
    },
  });
};

export const config = {
  matcher: [
    {
      source: "/((?!api|_next/static|_next/image|favicon.ico).*)",
      missing: [
        { type: "header", key: "next-router-prefetch" },
        { type: "header", key: "next-action" },
        { type: "header", key: "purpose", value: "prefetch" },
      ],
    },
  ],
};
nocodehummel commented 8 months ago

In case the middleware has some security responsibility adding { type: "header", key: "next-action" } will bypass the middleware.

jameslporter commented 7 months ago

@nicholas-yo thank you so much for sharing that, it resolved my issue! Lost over a day trying to get this working with my existing RHF forms with vanilla actions and useFormState with no luck. Setup next-safe-actions only to also end up getting undefined response even though I could finally see the data coming back whereas vanilla actions were returning an empty body. I have middleware setup for supertokens and adding that missing block got it working as expected!

@Ephem - can you try this and see if it resolves your problem, and close this out?

This kind of issue reminds me sometimes it's good to have a spare dummy project to test things out when hitting problems. Gives a sanity check as to whether you are doing something wrong, the system is broken, or something specific to your project configuration as is the case here.

IsakFriisJespersen2 commented 7 months ago

Thx @nicholas-yo! Solved my problem as well. Can adding this give any other side effects to the applications?

ArianHamdi commented 7 months ago

guys, i don't know if this is the best approach but adding the header "next-action" to "missing" in the config has solved the problem, for me.

my middleware file looks like this:

import { NextRequest, NextResponse } from "next/server";
import { generateCSP } from "./lib/generate-csp";
import { generateNonce } from "./lib/generate-nonce";
import { directives } from "./config/directives";

export const middleware = (req: NextRequest) => {
  const nonce = generateNonce();

  const cspHeader = generateCSP(directives({ nonce }));

  const requestHeaders = new Headers(req.headers);

  requestHeaders.set("x-nonce", nonce);

  requestHeaders.set("content-security-policy", cspHeader);

  return NextResponse.next({
    headers: requestHeaders,
    request: {
      headers: requestHeaders,
    },
  });
};

export const config = {
  matcher: [
    {
      source: "/((?!api|_next/static|_next/image|favicon.ico).*)",
      missing: [
        { type: "header", key: "next-router-prefetch" },
        { type: "header", key: "next-action" },
        { type: "header", key: "purpose", value: "prefetch" },
      ],
    },
  ],
};

You saved my day <3

mdwekat commented 6 months ago

I ended up implementing @nicholas-yo’s solution. However, I’m concerned this approach might bypass any middleware that handles security responsibilities. Could this be a potential issue?

jithujoshyjy commented 5 months ago

I was previously getting undefined thrown when calling a server action; on upgrading nextjs to v14.2.3 the following error occurs. This might be related.

react-server-dom-webpack-client.browser.development.js:2131 Uncaught (in promise) Error: Connection closed.
    at close (react-server-dom-webpack-client.browser.development.js:2131:31)
    at progress (react-server-dom-webpack-client.browser.development.js:2148:7)

Before upgrading, the above proposed solution didn't work for me; I had to remove the middleware.ts file now after the upgrade, even if I remove the middleware file, this happens instead.

jithujoshyjy commented 5 months ago

This error and #61995 seem to occur at least for me only when the browser dev tools is up; anyone else noticed this?

Error image

No Error image bringing up the dev tools and checking the console after form submit which fires the action shows the expected result image

jameslporter commented 5 months ago

Try removing all middleware. That's ultimately what resolved the issue for me. Self hosted fwiw.

Jim

On Sat, Jun 8, 2024, 12:37 PM Jithu Joshy Jy @.***> wrote:

This error and #61995 https://github.com/vercel/next.js/issues/61995 seem to occur at least for me only when the browser dev tools is up; anyone else noticed this?

Error image.png (view on web) https://github.com/vercel/next.js/assets/64597606/f15f934b-4527-4c88-9f30-b84db87e7d20

No Error image.png (view on web) https://github.com/vercel/next.js/assets/64597606/880d5c38-6e0c-452a-85a0-3d3579091ec2 bringing up the dev tools and checking the console after form submit which fires the action shows the expected result image.png (view on web) https://github.com/vercel/next.js/assets/64597606/5b5a7935-932f-4234-8fb3-64d9c275d1f5

— Reply to this email directly, view it on GitHub https://github.com/vercel/next.js/issues/50659#issuecomment-2156115658, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAG5UAE5Y4CJJ5LRCE5RZDZGM6OVAVCNFSM6AAAAAAYXPQC7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJWGEYTKNRVHA . You are receiving this because you commented.Message ID: <vercel/next. @.***>

jithujoshyjy commented 5 months ago

Try removing all middleware. That's ultimately what resolved the issue for me. Self hosted fwiw. Jim On Sat, Jun 8, 2024, 12:37 PM Jithu Joshy Jy @.> wrote: This error and #61995 <#61995> seem to occur at least for me only when the browser dev tools is up; anyone else noticed this? Error image.png (view on web) https://github.com/vercel/next.js/assets/64597606/f15f934b-4527-4c88-9f30-b84db87e7d20 No Error image.png (view on web) https://github.com/vercel/next.js/assets/64597606/880d5c38-6e0c-452a-85a0-3d3579091ec2 bringing up the dev tools and checking the console after form submit which fires the action shows the expected result image.png (view on web) https://github.com/vercel/next.js/assets/64597606/5b5a7935-932f-4234-8fb3-64d9c275d1f5 — Reply to this email directly, view it on GitHub <#50659 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAG5UAE5Y4CJJ5LRCE5RZDZGM6OVAVCNFSM6AAAAAAYXPQC7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJWGEYTKNRVHA . You are receiving this because you commented.Message ID: <vercel/next. @.>

How are you handling authentication? I don't want to add auth logic to all the pages.

tkrotoff commented 5 months ago

@Laecherlich [...] return NextResponse.next({ headers: requestHeaders, request: { headers: requestHeaders, }, }); [...] Without setting the headers it works perfectly.

In my opinion the proper solution is to create headers dedicated for the response instead of reusing the ones from the request, example:

export function middleware(request: NextRequest) {
  // Do not reuse the request headers in the response
  // Why? It will break server actions
  // See https://github.com/vercel/next.js/issues/50659#issuecomment-2157601329
  const responseHeaders = new Headers();

  addSomeHeadersToRequest(request);
  addSomeHeadersToResponse(request, responseHeaders);

  if (...) {
    return NextResponse.redirect(url, { headers: responseHeaders });
  }

  if (...) {
    return NextResponse.rewrite(url, { headers: responseHeaders });
  }

  addCSPHeaderToResponse(request, responseHeaders);

  return NextResponse.next({ headers: responseHeaders });
}



This approach is better than ignoring server actions requests in the middleware because it is too tight to Next.js internals and might break in the future.

@nicholas-yo adding the header "next-action" to "missing" in the config has solved the problem for me.

export const config = {
  matcher: [
    {
      source: '...',
      missing: [
        { type: 'header', key: 'next-action' },
        ...
      ],
    },
  ],
};
Tim0401 commented 5 months ago

In my case, the same issue occurred when a Server Actions request was intercepted by the Web Application Firewall. We are trying to determine this by setting a cookie in the server action to indicate that the call was successful, but this would not be an ideal solution.

simony commented 5 months ago

In my case, the same issue occurred when a Server Action failed due to "Vercel Runtime 413 Error: the response body is too large (> 4.5MB)." I'm getting undefined return value from my await ... instead of an exception, which like stated before makes it impossible to know if the undefined was returned by my Server Action or due to a failure (like in this case)

ioslh commented 5 months ago

In my case, the problem is caused by conflict content-type returned from server actions, which leads to isFlightResponse check failed.

image

Solution

Hope nextjs official can fix this, but before their solution, we can fixed it by this:

const fixContentType = 'text/x-component'

export function middleware(request: NextRequest) {
  const rewritedHeaders = new Headers(request.headers);
  const response =  NextResponse.next({
    headers: rewritedHeaders,
  });
  if (process.env.NODE_ENV === 'development') {
    // this only occurs during development
    if (response.headers.get('accept') === fixContentType) {
      response.headers.set('content-type', fixContentType)
    }
  }
  return response
}
taylor-lindores-reeves commented 4 months ago

Just spent ages trying to figure out why my server action was returning undefined until I came across this article. Can someone explain the logic behind this? A simple request header change in middleware should not complete mess up server actions... Here is my middleware:

export async function middleware(req: NextRequest) {
    req.headers.append("x-auth-token", `Bearer ${env.X_AUTH_TOKEN}`);
    return NextResponse.next({
        headers: req.headers,
    });
}
kevscript commented 4 months ago

Just spent ages trying to figure out why my server action was returning undefined until I came across this article. Can someone explain the logic behind this? A simple request header change in middleware should not complete mess up server actions... Here is my middleware:

export async function middleware(req: NextRequest) {
  req.headers.append("x-auth-token", `Bearer ${env.X_AUTH_TOKEN}`);
  return NextResponse.next({
      headers: req.headers,
  });
}

Running into the same problem when adding custom headers. Server actions returning undefined when this middleware is active. Without it, server actions return the expected response...

export function middleware(request: NextRequest) {
  const headers = new Headers(request.headers);
  headers.set("x-current-pathname", request.nextUrl.pathname);
  headers.set(
    "x-current-search-params",
    request.nextUrl.searchParams.toString()
  );
  return NextResponse.next({ headers });
}

Seems like editing headers changes the Content-Type. Using the following solution did fix it. What a mess honestly. Does it have any security implications?

In my case, the problem is caused by conflict content-type returned from server actions, which leads to [isFlightResponse]

const fixContentType = 'text/x-component'

export function middleware(request: NextRequest) {
  const rewritedHeaders = new Headers(request.headers);
  const response =  NextResponse.next({
    headers: rewritedHeaders,
  });
  if (process.env.NODE_ENV === 'development') {
    // this only occurs during development
    if (response.headers.get('accept') === fixContentType) {
      response.headers.set('content-type', fixContentType)
    }
  }
  return response
}
taylor-lindores-reeves commented 4 months ago

Subtle difference, but this is what worked for me. Do not include headers: requestHeaders, but use request: { ... } instead.

import { env } from "@/env";
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";

export async function middleware(req: NextRequest) {
    const requestHeaders = new Headers(req.headers);
    requestHeaders.set("x-auth-token", `Bearer ${env.AUTH_TOKEN}`);

    const response = NextResponse.next({
        // !-- do not do this (next line) as it will break server actions
        // !-- headers: requestHeaders,
        // instead, do this
        request: {
            headers: requestHeaders,
        },
    });
    return response;
       }
avalero commented 4 months ago

With this solution formData stops working when passed to a Server Action

const fixContentType = 'text/x-component'

export function middleware(request: NextRequest) {
  const rewritedHeaders = new Headers(request.headers);
  const response =  NextResponse.next({
    headers: rewritedHeaders,
  });
  if (process.env.NODE_ENV === 'development') {
    // this only occurs during development
    if (response.headers.get('accept') === fixContentType) {
      response.headers.set('content-type', fixContentType)
    }
  }
  return response
}
ezeparziale commented 2 months ago

This is the best solution?

export const config = {
  matcher: [
    {
      source: "/((?!api|_next/static|_next/image|favicon.ico).*)",
      missing: [
        { type: "header", key: "next-router-prefetch" },
        { type: "header", key: "next-action" },
        { type: "header", key: "purpose", value: "prefetch" },
      ],
    },
  ],
};
Mrowa96 commented 2 months ago

After some research - we have to differentiate between request and response headers correctly.

To set some custom response headers we can do it like this:

export function middleware(request: NextRequest) {
  // We shouldn't clone request headers here because we are setting response headers.
  const rewritedHeaders = new Headers();

  return  NextResponse.next({
    headers: rewritedHeaders,
  });
}

To set request headers:

export function middleware(request: NextRequest) {
  // We have to clone request headers here because we are passing them further
  const rewritedHeaders = new Headers(request.headers);

  return  NextResponse.next({
    request: {
       headers: rewritedHeaders,
    }
  });
}

This way Server actions are working fine (at least on newest Next.js 14.2.8) and our headers are set correctly.

cesarochoa2006 commented 2 months ago

After some research - we have to differentiate between request and response headers correctly.

To set some custom response headers we can do it like this:

export function middleware(request: NextRequest) {
  // We shouldn't clone request headers here because we are setting response headers.
  const rewritedHeaders = new Headers();

  return  NextResponse.next({
    headers: rewritedHeaders,
  });
}

To set request headers:

export function middleware(request: NextRequest) {
  // We have to clone request headers here because we are passing them further
  const rewritedHeaders = new Headers(request.headers);

  return  NextResponse.next({
    request: {
       headers: rewritedHeaders,
    }
  });
}

This way Server actions are working fine (at least on newest Next.js 14.2.8) and our headers are set correctly.

Thank you! I've also have spent long time on this issue and this fix solves it correctly

x0y-gt commented 1 month ago

guys, i don't know if this is the best approach but adding the header "next-action" to "missing" in the config has solved the problem, for me.

my middleware file looks like this:

import { NextRequest, NextResponse } from "next/server";
import { generateCSP } from "./lib/generate-csp";
import { generateNonce } from "./lib/generate-nonce";
import { directives } from "./config/directives";

export const middleware = (req: NextRequest) => {
  const nonce = generateNonce();

  const cspHeader = generateCSP(directives({ nonce }));

  const requestHeaders = new Headers(req.headers);

  requestHeaders.set("x-nonce", nonce);

  requestHeaders.set("content-security-policy", cspHeader);

  return NextResponse.next({
    headers: requestHeaders,
    request: {
      headers: requestHeaders,
    },
  });
};

export const config = {
  matcher: [
    {
      source: "/((?!api|_next/static|_next/image|favicon.ico).*)",
      missing: [
        { type: "header", key: "next-router-prefetch" },
        { type: "header", key: "next-action" },
        { type: "header", key: "purpose", value: "prefetch" },
      ],
    },
  ],
};

This was so frustrating... took me a day to find this to solve it, thanks!

Jsox commented 1 month ago

Here is my solution for headers in middleware

import { NextResponse } from 'next/server';
import type { NextRequest } from 'next/server';

export function middleware(request: NextRequest) {
  const response = NextResponse.next({
    request: {
      headers: request.headers,
    },
  });

  response.headers.set('x-current-path', request.nextUrl.pathname);

  return response;
}
shaedmorgan commented 1 month ago

The solution above worked for me

Here is my solution for headers in middleware

import { NextResponse } from 'next/server';
import type { NextRequest } from 'next/server';

export function middleware(request: NextRequest) {
  const response = NextResponse.next({
    request: {
      headers: request.headers,
    },
  });

  response.headers.set('x-current-path', request.nextUrl.pathname);

  return response;
}