Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,6 @@ Review the `README.md` and `CONTRIBUTING.md` for all relevant repository informa
- Test startup is slow by design — each test file starts a real Harper instance and waits for Next.js to build (up to 2 minutes). A slow start is not a failure.
- The ISR cache tests in `integrationTests/next-16.pw.ts` are intentionally skipped; `CacheHandler.cts` is a work in progress.
- `next-16-static-data` is run by two test files: `next-16-static-data.pw.ts` on the default VM module loader (where Harper's component `harper` allowlist omits `flushDatabases`, so the plugin's pre-build flush is a no-op and a read-only build child can't see unflushed writes) and `next-16-static-data-native.pw.ts` under `applications.moduleLoader: native`, where the flush does run. The pair is what pins that behavior down — keep both.
- `next-16-mounted` covers an application served under a Harper `urlPath`. Its assertions are `test.describe.fixme` and must be un-skipped once harper's `Request.withNodeAdapter()` presents a faithful Node request/response — see HarperFast/nextjs#61 and the reproducers in `~/dev/scripts/harper-node-adapter-repro`. Against today's harper an adapted request 500s on `headers.hasOwnProperty` and a missing `appendHeader`/`_implicitHeader`, and any response larger than the adapter's 16 KB buffer stalls with no error. Every other fixture is unmounted, so nothing rewrites its URL, it keeps the direct hand-off to Next.js, and it is unaffected.
- CI is currently disabled (`if: false` in `.github/workflows/integration-tests.yml`). Run tests locally.
- The `page`-based tests need Playwright's browser binaries (`npx playwright install chromium`); without them they fail instantly with `browserType.launch: Executable doesn't exist`. The `request`-based tests do not.
1 change: 1 addition & 0 deletions fixtures/next-16-mounted/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock=false
3 changes: 3 additions & 0 deletions fixtures/next-16-mounted/app/about/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <h1>Mounted About</h1>;
}
6 changes: 6 additions & 0 deletions fixtures/next-16-mounted/app/api/echo/route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const dynamic = 'force-dynamic';

export async function GET(request) {
const url = new URL(request.url);
return Response.json({ pathname: url.pathname, search: url.search });
}
11 changes: 11 additions & 0 deletions fixtures/next-16-mounted/app/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export const metadata = {
title: 'Harper - Mounted Next.js App',
};

export default function RootLayout({ children }) {
return (
<html>
<body>{children}</body>
</html>
);
}
3 changes: 3 additions & 0 deletions fixtures/next-16-mounted/app/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <h1>Mounted Home</h1>;
}
3 changes: 3 additions & 0 deletions fixtures/next-16-mounted/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
'@harperfast/nextjs':
package: '@harperfast/nextjs'
urlPath: /mounted
3 changes: 3 additions & 0 deletions fixtures/next-16-mounted/next.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { withHarper } from '@harperfast/nextjs';

export default withHarper({});
16 changes: 16 additions & 0 deletions fixtures/next-16-mounted/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"name": "next-16-mounted",
"private": true,
"scripts": {
"dev": "next dev",
"build": "next build",
"start": "next start",
"lint": "next lint"
},
"dependencies": {
"@harperfast/nextjs": "file:../../",
"react": "^19",
"react-dom": "^19",
"next": "^16"
}
}
47 changes: 47 additions & 0 deletions integrationTests/next-16-mounted.pw.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { fixture } from './fixture.ts';

const { test, expect } = fixture('next-16-mounted');

// Un-skip once harper's `Request.withNodeAdapter()` can serve Next.js — today an adapted request 500s
// on `headers.hasOwnProperty` and a missing `appendHeader`/`_implicitHeader`, and any response over
// the adapter's 16 KB buffer stalls. Skipped rather than left red because with CI disabled a green
// local run is this repo's only regression signal. See HarperFast/nextjs#61.
test.describe.fixme('served under a urlPath mount', () => {
test('mount root reaches the Next.js home page', async ({ request, harper }) => {
const response = await request.get(`${harper.httpURL}/mounted`);
expect(response.status()).toBe(200);
expect(await response.text()).toContain('Mounted Home');
});

test('a nested page under the mount reaches its Next.js route', async ({ request, harper }) => {
const response = await request.get(`${harper.httpURL}/mounted/about`);
expect(response.status()).toBe(200);
expect(await response.text()).toContain('Mounted About');
});

test('Next.js sees the mount-relative path, not the requested one', async ({ request, harper }) => {
const response = await request.get(`${harper.httpURL}/mounted/api/echo`);
expect(response.status()).toBe(200);
expect(await response.json()).toEqual({ pathname: '/api/echo', search: '' });
});

test('the query string survives mount stripping', async ({ request, harper }) => {
const response = await request.get(`${harper.httpURL}/mounted/api/echo?q=1`);
expect(response.status()).toBe(200);
expect(await response.json()).toEqual({ pathname: '/api/echo', search: '?q=1' });
});

// Asserts on the server-rendered markup only: Next.js still emits its `/_next/*` asset URLs at the
// root, outside the mount, until the app is built with a matching `basePath`.
test('the mount root renders in a browser', async ({ page, harper }) => {
await page.goto(`${harper.httpURL}/mounted`);
await expect(page.locator('h1')).toHaveText('Mounted Home');
});
});

// Outside the fixme block: nothing rewrites this URL, so it never reaches the adapter.
test('paths outside the mount are not served by Next.js', async ({ request, harper }) => {
const response = await request.get(`${harper.httpURL}/about`);
expect(response.status()).toBe(404);
expect(await response.text()).not.toContain('Mounted About');
});
42 changes: 38 additions & 4 deletions src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,12 +394,46 @@ async function serve(scope: Scope, config: NextPluginConfig, next: NextPackage)

const requestHandler = app.getRequestHandler();

let warnedMissingNodeAdapter = false;

scope.server?.http?.(
(request, next) => {
return request._nodeResponse === undefined
? next(request)
: // @ts-expect-error - Not sure when the IncomingMessage.url could be undefined ; need to dig into it.
requestHandler(request._nodeRequest, request._nodeResponse, urlParse(request._nodeRequest.url, true));
// `== null`, not `=== undefined`: Harper's Bun and uWS requests carry a null `_nodeResponse`,
// and neither implements the Node adapter used below.
if (request._nodeResponse == null) return next(request);
// Harper's router strips an application's urlPath mount by proxying the Harper `Request`, so the
// Node request underneath it still carries the un-stripped URL. Only a request middleware
// rewrote needs the adapter, which presents the Request's method/url/headers over that Node
// request; everything else keeps the cheaper direct hand-off.
if (request.url !== request._nodeRequest.url) {
if (typeof request.withNodeAdapter === 'function') {
return request
.withNodeAdapter((nodeRequest, nodeResponse) =>
// @ts-expect-error - Not sure when the IncomingMessage.url could be undefined ; need to dig into it.
requestHandler(nodeRequest, nodeResponse, urlParse(nodeRequest.url, true))
)
.then((response) => {
// Required by withNodeAdapter: a connection reset after the headers are sent destroys
// this stream with an error, which Node throws as an uncaught exception without a
// listener. Harper's own pipe already warns about the error, so this only has to
// exist, not report.
response.body.on('error', (error) =>
scope.logger.debug?.(`Next.js response stream error for ${request.pathname}: `, error)
);
Comment on lines +420 to +422

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If the Next.js response has no body (for example, with 204 No Content, 304 Not Modified, or HEAD requests), response.body will be null or undefined. Calling response.body.on('error', ...) directly will throw a TypeError: Cannot read properties of null (reading 'on') immediately when the promise resolves, causing these requests to fail with a 500 error.

Using optional chaining (response.body?.on) prevents this crash for empty responses.

Suggested change
response.body.on('error', (error) =>
scope.logger.debug?.(`Next.js response stream error for ${request.pathname}: `, error)
);
response.body?.on('error', (error) =>
scope.logger.debug?.(`Next.js response stream error for ${request.pathname}: `, error)
);

return response;
});
}
if (!warnedMissingNodeAdapter) {
warnedMissingNodeAdapter = true;
scope.logger.warn?.(
'This version of Harper does not provide Request.withNodeAdapter, so Next.js receives the URL as it ' +
`arrived rather than the ${request.pathname} Harper resolved it to. An application mounted at a ` +
'urlPath will not route correctly; upgrade Harper or serve the application at the root.'
);
}
}
// @ts-expect-error - Not sure when the IncomingMessage.url could be undefined ; need to dig into it.
return requestHandler(request._nodeRequest, request._nodeResponse, urlParse(request._nodeRequest.url, true));
},
{ runFirst: config.runFirst, port: config.port, securePort: config.securePort }
);
Expand Down
Loading