Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes Node adapter receiving a request body #4023

Merged
merged 2 commits into from
Jul 22, 2022
Merged
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
6 changes: 6 additions & 0 deletions .changeset/smooth-seahorses-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'astro': patch
'@astrojs/node': patch
---

Fixes Node adapter to accept a request body
30 changes: 28 additions & 2 deletions packages/astro/src/core/app/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ import { App } from './index.js';

const clientAddressSymbol = Symbol.for('astro.clientAddress');

function createRequestFromNodeRequest(req: IncomingMessage): Request {
function createRequestFromNodeRequest(req: IncomingMessage, body?: string): Request {
let url = `http://${req.headers.host}${req.url}`;
let rawHeaders = req.headers as Record<string, any>;
const entries = Object.entries(rawHeaders);
let request = new Request(url, {
method: req.method || 'GET',
headers: new Headers(entries),
body
});
if (req.socket.remoteAddress) {
if (req.socket?.remoteAddress) {
Reflect.set(request, clientAddressSymbol, req.socket.remoteAddress);
}
return request;
Expand All @@ -26,6 +27,31 @@ export class NodeApp extends App {
return super.match(req instanceof Request ? req : createRequestFromNodeRequest(req));
}
render(req: IncomingMessage | Request) {
if('on' in req) {
let body: string | undefined = undefined;
let reqBodyComplete = new Promise((resolve, reject) => {
req.on('data', d => {
if(body === undefined) {
body = '';
}
if(d instanceof Buffer) {
body += d.toString('utf-8');
} else if(typeof d === 'string') {
body += d;
}
});
req.on('end', () => {
resolve(body);
});
req.on('error', err => {
reject(err);
});
});

return reqBodyComplete.then(() => {
return super.render(req instanceof Request ? req : createRequestFromNodeRequest(req, body));
});
}
return super.render(req instanceof Request ? req : createRequestFromNodeRequest(req));
}
}
Expand Down
6 changes: 4 additions & 2 deletions packages/integrations/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@
"scripts": {
"build": "astro-scripts build \"src/**/*.ts\" && tsc",
"build:ci": "astro-scripts build \"src/**/*.ts\"",
"dev": "astro-scripts dev \"src/**/*.ts\""
"dev": "astro-scripts dev \"src/**/*.ts\"",
"test": "mocha --exit --timeout 20000 test/"
},
"dependencies": {
"@astrojs/webapi": "^0.12.0"
},
"devDependencies": {
"astro": "workspace:*",
"astro-scripts": "workspace:*"
"astro-scripts": "workspace:*",
"node-mocks-http": "^1.11.0"
}
}
31 changes: 19 additions & 12 deletions packages/integrations/node/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,28 @@ export function createExports(manifest: SSRManifest) {
const app = new NodeApp(manifest);
return {
async handler(req: IncomingMessage, res: ServerResponse, next?: (err?: unknown) => void) {
const route = app.match(req);
try {
const route = app.match(req);

if (route) {
try {
const response = await app.render(req);
await writeWebResponse(res, response);
} catch (err: unknown) {
if (next) {
next(err);
} else {
throw err;
if (route) {
try {
const response = await app.render(req);
await writeWebResponse(res, response);
} catch (err: unknown) {
if (next) {
next(err);
} else {
throw err;
}
}
} else if (next) {
return next();
}
} catch(err: unknown) {
if(!res.headersSent) {
res.writeHead(500, `Server error`);
res.end();
}
} else if (next) {
return next();
}
},
};
Expand Down
37 changes: 37 additions & 0 deletions packages/integrations/node/test/api-route.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import nodejs from '../dist/index.js';
import { loadFixture, createRequestAndResponse, toPromise } from './test-utils.js';
import { expect } from 'chai';


describe('API routes', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;

before(async () => {
fixture = await loadFixture({
root: './fixtures/api-route/',
experimental: {
ssr: true,
},
adapter: nodejs(),
});
await fixture.build();
});

it('Can get the request body', async () => {
const { handler } = await import('./fixtures/api-route/dist/server/entry.mjs');

let { req, res, done } = createRequestAndResponse({
method: 'POST',
url: '/recipes'
});

handler(req, res);
req.send(JSON.stringify({ id: 2 }));

let [ buffer ] = await done;
let json = JSON.parse(buffer.toString('utf-8'));
expect(json.length).to.equal(1);
expect(json[0].name).to.equal('Broccoli Soup');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@test/nodejs-api-route",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*",
"@astrojs/node": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@

export async function post({ request }) {
let body = await request.json();
const recipes = [
{
id: 1,
name: 'Potato Soup'
},
{
id: 2,
name: 'Broccoli Soup'
}
];

let out = recipes.filter(r => {
return r.id === body.id;
});

return new Response(JSON.stringify(out), {
headers: {
'Content-Type': 'application/json'
}
});
}
41 changes: 41 additions & 0 deletions packages/integrations/node/test/test-utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { loadFixture as baseLoadFixture } from '../../../astro/test/test-utils.js';
import httpMocks from 'node-mocks-http';
import { EventEmitter } from 'events';

/**
* @typedef {import('../../../astro/test/test-utils').Fixture} Fixture
*/

export function loadFixture(inlineConfig) {
if (!inlineConfig || !inlineConfig.root)
throw new Error("Must provide { root: './fixtures/...' }");

// resolve the relative root (i.e. "./fixtures/tailwindcss") to a full filepath
// without this, the main `loadFixture` helper will resolve relative to `packages/astro/test`
return baseLoadFixture({
...inlineConfig,
root: new URL(inlineConfig.root, import.meta.url).toString(),
});
}

export function createRequestAndResponse(reqOptions) {
let req = httpMocks.createRequest(reqOptions);

let res = httpMocks.createResponse({
eventEmitter: EventEmitter,
req
});

let done = toPromise(res);

return { req, res, done };
}

export function toPromise(res) {
return new Promise(resolve => {
res.on('end', () => {
let chunks = res._getChunks();
resolve(chunks);
});
});
}
Loading