Skip to content

Conversation

@unstubbable
Copy link
Contributor

Alternative for #86740.

This should avoid running into these issues:

request.allHeaders: Target page, context or browser has been closed

The sync headers() method is sufficient for these use cases:

Note that this method does not return security-related headers, including cookie-related ones. You can use request.allHeaders() for complete list of headers that include cookie information.

Note: The other occurrences of request.allHeaders() we have are fine because they're used in page.route() which does handle asynchronous callbacks.

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. tests labels Dec 2, 2025
@ijjk
Copy link
Member

ijjk commented Dec 2, 2025

Failing test suites

Commit: 92af4e8 | About building and testing Next.js

pnpm test-start test/e2e/app-dir/app-basepath/index.test.ts (job)

  • app dir - basepath > should only make a single RSC call to the current page (/base/refresh?foo=bar) (DD)
Expand output

● app dir - basepath › should only make a single RSC call to the current page (/base/refresh?foo=bar)

expect(received).toEqual(expected) // deep equality

- Expected  - 0
+ Received  + 1

  Array [
    StringContaining "http://localhost:37225/base/refresh?foo=bar",
+   "http://localhost:37225/base/refresh?foo=bar&_rsc=1yllp",
  ]

  92 |       await browser.waitForIdleNetwork()
  93 |       await retry(async () => {
> 94 |         expect(rscRequests).toEqual([
     |                             ^
  95 |           expect.stringContaining(`${next.url}${path}`),
  96 |         ])
  97 |       })

  at toEqual (e2e/app-dir/app-basepath/index.test.ts:94:29)
  at fn (lib/next-test-utils.ts:797:20)
  at e2e/app-dir/app-basepath/index.test.ts:93:7

This should avoid running into these issues:

```
request.allHeaders: Target page, context or browser has been closed
```

The sync `headers()` method is sufficient for these use cases:

> Note that this method does not return security-related headers,
> including cookie-related ones. You can use `request.allHeaders()` for
> complete list of headers that include cookie information.

Alternative for #86740.
@unstubbable unstubbable force-pushed the hl/avoid-request-all-headers branch from 135b74f to 92af4e8 Compare December 2, 2025 23:14
@unstubbable
Copy link
Contributor Author

unstubbable commented Dec 2, 2025

This seems to have uncovered either a bug or a wrong expectation with cacheComponents enabled. There are actually two RSC requests with different refetch markers in the router state tree being sent:

  • ["",{"children":["refresh",{"children":["__PAGE__",{},null,"refetch"]},null,null]},null,null]
  • ["",{"children":["refresh",{"children":["__PAGE__",{},null,null]},null,null]},null,"refetch",true]

@ztanner Is this expected?

Repro:

__NEXT_CACHE_COMPONENTS=true NEXT_SKIP_ISOLATE=1 pnpm test-start test/e2e/app-dir/app-basepath/index.test.ts -t 'foo=bar'

@ztanner
Copy link
Member

ztanner commented Dec 2, 2025

@unstubbable that looks suspiciously like the lazy fetch we do in layout-router when something about the tree mismatches and we don't have anything to render. If that's the case, I think all cases of the lazy fetch occurring is technically a bug (especially in cache components) so we'd need to figure out what's wrong there.

@mischnic
Copy link
Contributor

mischnic commented Dec 3, 2025

When I tried it, .headers() didn't include the custom RSC headers.

@unstubbable
Copy link
Contributor Author

When I tried it, .headers() didn't include the custom RSC headers.

Hm, worked just fine here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

created-by: Next.js team PRs by the Next.js team. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants