-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add Express and Lambda adapters #283
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,12 +2,14 @@ | |||||
| "name": "http-playground-server", | ||||||
| "version": "0.0.1", | ||||||
| "type": "module", | ||||||
| "main": "dist/server.js", | ||||||
| "main": "dist/adapters/express/index.js", | ||||||
| "repository": "https://github.com/ryo8000/http-playground-server.git", | ||||||
| "author": "Ryo Hasegawa", | ||||||
| "license": "MIT", | ||||||
| "scripts": { | ||||||
| "build": "tsc", | ||||||
| "build:lambda:function": "yarn build && cp package.json dist/ && zip -r lambda-function.zip dist/", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
A simple improvement for the zip structure is to
Suggested change
|
||||||
| "build:lambda:layer": "mkdir -p lambda-layer/nodejs && cp -r node_modules lambda-layer/nodejs && cd lambda-layer && zip -r ../lambda-layer.zip . && cd .. && rm -rf lambda-layer", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Package only runtime dependencies in the Lambda layer. Copying the repo’s whole 🤖 Prompt for AI Agents |
||||||
| "dev": "nodemon", | ||||||
| "test": "jest --coverage", | ||||||
| "test:api": "newman run tests/api/api-test-collection.json -e tests/api/environment.json --reporters cli", | ||||||
|
|
@@ -24,6 +26,7 @@ | |||||
| }, | ||||||
| "devDependencies": { | ||||||
| "@eslint/js": "10.0.1", | ||||||
| "@types/aws-lambda": "8.10.149", | ||||||
| "@types/cookie-parser": "1.4.10", | ||||||
| "@types/cors": "2.8.19", | ||||||
| "@types/express": "5.0.6", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| import http from 'node:http'; | ||
| import { createServer } from 'node:http'; | ||
| import type { | ||
| APIGatewayProxyEventV2, | ||
| APIGatewayProxyStructuredResultV2, | ||
| Handler, | ||
| } from 'aws-lambda'; | ||
| import { app } from '../express/app.js'; | ||
|
|
||
| /** Hop-by-hop headers that must not be forwarded to the loopback request. */ | ||
| const HOP_BY_HOP_HEADERS = new Set([ | ||
| 'connection', | ||
| 'keep-alive', | ||
| 'transfer-encoding', | ||
| 'te', | ||
| 'trailer', | ||
| 'upgrade', | ||
| 'proxy-authorization', | ||
| 'proxy-authenticate', | ||
| ]); | ||
|
|
||
| /** | ||
| * Starts the Express app on a random loopback port and resolves with the port number. | ||
| * Called once on cold start; subsequent invocations reuse the cached promise. | ||
| */ | ||
| const listeningPromise: Promise<number> = new Promise((resolve, reject) => { | ||
| const server = createServer(app); | ||
| server.listen(0, '127.0.0.1', () => { | ||
| const address = server.address(); | ||
| if (address === null || typeof address === 'string') { | ||
| reject(new Error('Unexpected server address format')); | ||
| return; | ||
| } | ||
| resolve(address.port); | ||
| }); | ||
| server.on('error', reject); | ||
| }); | ||
|
Comment on lines
+26
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing The 🐛 Proposed fix to set RUNTIME before starting the server+// Set runtime environment for Lambda-specific behavior in routes
+process.env['RUNTIME'] = 'lambda';
+
/**
* Starts the Express app on a random loopback port and resolves with the port number.
* Called once on cold start; subsequent invocations reuse the cached promise.
*/
const listeningPromise: Promise<number> = new Promise((resolve, reject) => {🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * Converts an API Gateway HTTP API v2 event into a loopback HTTP request | ||
| * against the locally running Express server, and maps the response back | ||
| * to an API Gateway proxy result. | ||
| * | ||
| * @param event - The API Gateway v2 proxy event. | ||
| * @returns A structured API Gateway proxy result. | ||
| */ | ||
| export const handler: Handler<APIGatewayProxyEventV2, APIGatewayProxyStructuredResultV2> = async ( | ||
| event, | ||
| ) => { | ||
| const port = await listeningPromise; | ||
|
|
||
| const method = event.requestContext.http.method; | ||
| const path = event.rawPath + (event.rawQueryString ? `?${event.rawQueryString}` : ''); | ||
|
|
||
| const bodyBuffer: Buffer = event.body | ||
| ? Buffer.from(event.body, event.isBase64Encoded ? 'base64' : 'utf-8') | ||
| : Buffer.alloc(0); | ||
|
|
||
| // Build forwarded headers, skipping hop-by-hop headers and recalculating content-length. | ||
| const forwardHeaders: Record<string, string> = {}; | ||
| for (const [key, value] of Object.entries(event.headers ?? {})) { | ||
| const lower = key.toLowerCase(); | ||
| if (!HOP_BY_HOP_HEADERS.has(lower) && value !== undefined) { | ||
| forwardHeaders[lower] = value; | ||
| } | ||
| } | ||
| if (bodyBuffer.length > 0) { | ||
| forwardHeaders['content-length'] = String(bodyBuffer.length); | ||
| } | ||
|
|
||
| try { | ||
| const { statusCode, headers, rawBody } = await new Promise<{ | ||
| statusCode: number; | ||
| headers: Record<string, string | string[]>; | ||
| rawBody: Buffer; | ||
| }>((resolve, reject) => { | ||
| const req = http.request( | ||
| { | ||
| hostname: '127.0.0.1', | ||
| port, | ||
| method, | ||
| path, | ||
| headers: forwardHeaders, | ||
| }, | ||
| (res) => { | ||
| const chunks: Buffer[] = []; | ||
| res.on('data', (chunk: Buffer) => chunks.push(chunk)); | ||
| res.on('end', () => { | ||
| const responseHeaders: Record<string, string | string[]> = {}; | ||
| for (const [key, value] of Object.entries(res.headers)) { | ||
| if (value !== undefined) { | ||
| responseHeaders[key] = value; | ||
| } | ||
| } | ||
| resolve({ | ||
| statusCode: res.statusCode ?? 200, | ||
| headers: responseHeaders, | ||
| rawBody: Buffer.concat(chunks), | ||
| }); | ||
| }); | ||
| res.on('error', reject); | ||
| }, | ||
| ); | ||
| req.on('error', reject); | ||
| if (bodyBuffer.length > 0) { | ||
| req.write(bodyBuffer); | ||
| } | ||
| req.end(); | ||
| }); | ||
|
|
||
| // Extract Set-Cookie headers into the cookies array (API Gateway v2 requirement). | ||
| const cookies: string[] = []; | ||
| const responseHeaders: Record<string, string> = {}; | ||
|
|
||
| for (const [key, value] of Object.entries(headers)) { | ||
| if (key.toLowerCase() === 'set-cookie') { | ||
| const cookieValues = Array.isArray(value) ? value : [value]; | ||
| cookies.push(...cookieValues); | ||
| } else { | ||
| responseHeaders[key] = Array.isArray(value) ? value.join(', ') : value; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| statusCode, | ||
| headers: responseHeaders, | ||
| body: rawBody.toString('utf-8'), | ||
| ...(cookies.length > 0 && { cookies }), | ||
| }; | ||
|
Comment on lines
+124
to
+129
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The response body is always converted to a utf-8 string. This will corrupt binary responses. The handler should check the const contentType = responseHeaders['content-type'] || '';
const isText = /^(text\/|application\/json)/.test(contentType);
return {
statusCode,
headers: responseHeaders,
body: rawBody.toString(isText ? 'utf8' : 'base64'),
isBase64Encoded: !isText,
...(cookies.length > 0 && { cookies }),
}; |
||
| } catch { | ||
| return { | ||
| statusCode: 500, | ||
| body: JSON.stringify({ error: { message: 'Internal server error' } }), | ||
| headers: { 'content-type': 'application/json' }, | ||
| }; | ||
| } | ||
|
Comment on lines
+130
to
+136
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This } catch (error) {
console.error('Error in Lambda handler:', error);
return {
statusCode: 500,
body: JSON.stringify({ error: { message: 'Internal server error' } }),
headers: { 'content-type': 'application/json' },
};
} |
||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,15 @@ errorRouter.all('/timeout', () => { | |
| return; | ||
| }); | ||
|
|
||
| errorRouter.all('/network', (req) => { | ||
| errorRouter.all('/network', (req, res) => { | ||
| if (process.env['RUNTIME'] === 'lambda') { | ||
| res | ||
| .status(400) | ||
| .json({ | ||
| error: { message: 'Network error simulation is not supported in this environment.' }, | ||
| }); | ||
| return; | ||
|
Comment on lines
+11
to
+17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The associated test for this logic in |
||
| } | ||
| // Intentionally destroy the connection | ||
| req.socket.destroy(); | ||
| return; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Keep the documented source layout in sync with the repo conventions.
This tree now tells contributors to put app/server code under
src/adapters/express/and shared modules undersrc/core/, but the current project conventions still point tosrc/app.ts,src/server.ts,src/env.ts,src/logger.ts, andsrc/routes/. Please resolve that mismatch in this PR so the repo has a single authoritative layout.Based on learnings: "Applies to src/app.ts : Main Express application setup with middleware chain and route registration must be in
src/app.ts", "Applies to src/routes/**/*.{ts,tsx} : All routes must be located insrc/routes/and export an Express Router", "Applies to src/logger.ts : Logger setup with development/production configurations must be insrc/logger.ts", "Applies to src/server.ts : HTTP server startup with graceful shutdown handling must be insrc/server.ts", and "Applies to src/env.ts : Configure environment variables insrc/env.tsincluding ENABLE_SHUTDOWN, LOG_LEVEL, MAX_DELAY, NODE_ENV, ORIGIN, PORT, KEEP_ALIVE_TIMEOUT, HEADERS_TIMEOUT, and REQUEST_TIMEOUT".🤖 Prompt for AI Agents