Skip to content

Commit 0dd219f

Browse files
committed
remove logging add test
1 parent 0495452 commit 0dd219f

4 files changed

Lines changed: 56 additions & 77 deletions

File tree

CLAUDE.md

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,15 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
1818

1919
### Core Services
2020
- **Express Router Service**: Main HTTP server handling compilation and cmake requests for Compiler Explorer
21+
- **HTTP Forwarder** (`src/services/http-forwarder.ts`): Direct HTTP forwarding to target URLs with proper header handling
2122
- **WebSocket Manager** (`src/services/websocket-manager.ts`): Robust WebSocket client with automatic reconnection, subscription management, and ping/pong keepalive
2223
- **AWS Clients** (`src/services/aws-clients.ts`): Pre-configured AWS SDK v3 clients for DynamoDB, S3, SQS, SSM, and STS
2324

2425
### Key Endpoints
2526
- `POST /api/compiler/:compilerid/compile` - Handles compilation requests for specific compilers
2627
- `POST /api/compiler/:compilerid/cmake` - Handles CMake build requests
28+
- `POST /:env/api/compiler/:compilerid/compile` - Environment-prefixed compilation requests (beta, staging)
29+
- `POST /:env/api/compiler/:compilerid/cmake` - Environment-prefixed CMake requests
2730
- `GET /healthcheck` - Health status including WebSocket connection state
2831

2932
### WebSocket Integration
@@ -51,5 +54,31 @@ The service maintains a persistent WebSocket connection to `wss://events.compile
5154
- 120 character line width
5255
- Strict TypeScript configuration with unused parameter/local detection
5356

54-
The codebase is a TypeScript replacement for an AWS Lambda function, designed to handle Compiler Explorer routing requests with WebSocket-based real-time communication.
55-
- always lint and check the code when you have finished code changes
57+
## Routing Architecture
58+
59+
The service supports two routing methods:
60+
61+
### Queue-based Routing (Default)
62+
- Requests are sent to SQS queues for processing
63+
- Results are returned via WebSocket connections
64+
- Supports automatic failover and load balancing
65+
- Used for most compilation requests
66+
67+
### URL-based Routing
68+
- Direct HTTP forwarding to specific target URLs
69+
- Configured via DynamoDB routing table
70+
- Used for specialized compilers (e.g., GPU compilers)
71+
- Handles HTTP header normalization to prevent protocol violations
72+
73+
### Important Notes
74+
- HTTP responses automatically remove conflicting headers (e.g., `transfer-encoding` when setting `content-length`)
75+
- Large responses (>1MB) are flagged with warnings due to ALB limits
76+
- CORS headers are automatically added to all responses
77+
78+
The codebase is a TypeScript replacement for an AWS Lambda function, designed to handle Compiler Explorer routing requests with both WebSocket-based real-time communication and direct HTTP forwarding.
79+
80+
## Development Guidelines
81+
- Always lint and check the code when you have finished code changes
82+
- Use debug-level logging for detailed troubleshooting information
83+
- Keep info-level logging for essential operational messages
84+
- Prefer unit tests for pure functions over integration tests where possible

src/compiler-explorer-router.ts

Lines changed: 5 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import {createHash} from 'node:crypto';
21
import express from 'express';
32
import type {Application, Request, Response} from 'express';
43
import {logger} from './lib/logger.js';
@@ -219,7 +218,7 @@ export class CompilerExplorerRouter {
219218
await this.resultWaiter.unsubscribe(guid);
220219

221220
try {
222-
logger.info(`Starting URL forwarding for ${compilerid} to ${routingInfo.target}`);
221+
logger.debug(`Starting URL forwarding for ${compilerid} to ${routingInfo.target}`);
223222
const response = await forwardToEnvironmentUrl(
224223
compilerid,
225224
routingInfo.target,
@@ -228,86 +227,43 @@ export class CompilerExplorerRouter {
228227
headers as Record<string, string | string[]>,
229228
);
230229

231-
logger.info(
230+
logger.debug(
232231
`Got response from forwardToEnvironmentUrl: status=${response.statusCode}, body length=${response.body.length}`,
233232
);
234233

235-
// Ensure CORS headers are present and clean up conflicting headers
234+
// Ensure CORS headers are present
236235
const responseHeaders: Record<string, string> = {
237236
...response.headers,
238237
'Access-Control-Allow-Origin': '*',
239238
'Access-Control-Allow-Methods': 'POST, GET, OPTIONS',
240239
'Access-Control-Allow-Headers': 'Content-Type, Accept, Authorization',
241240
};
242241

243-
// Remove transfer-encoding if present since we're setting content-length
244-
delete responseHeaders['transfer-encoding'];
245-
246242
// Check if response was already sent
247243
if (res.headersSent) {
248244
logger.error(`Headers already sent for ${compilerid}, cannot send response`);
249245
return;
250246
}
251247

252-
logger.info(
253-
`Sending response to client with status ${response.statusCode}, body length ${response.body.length}`,
254-
);
255-
256248
// Set content-length header explicitly to help proxies
257249
const bodyBuffer = Buffer.from(response.body);
258-
const bodyHash = createHash('md5').update(bodyBuffer).digest('hex');
259250
responseHeaders['content-length'] = bodyBuffer.length.toString();
260-
logger.info(`Response body is ${bodyBuffer.length} bytes, MD5: ${bodyHash}, setting content-length header`);
261251

262252
// Check if response is too large for some proxies (ALB limit is 1MB)
263253
if (bodyBuffer.length > 1000000) {
264254
logger.warn(`Response body size ${bodyBuffer.length} bytes exceeds 1MB - may cause ALB issues`);
265255
}
266256

267-
// Log response body type and sample
268-
if (typeof response.body === 'string') {
269-
const preview = response.body.length > 100 ? response.body.substring(0, 100) + '...' : response.body;
270-
logger.info(`Response body (string): ${preview}`);
271-
} else {
272-
const bodyType = typeof response.body;
273-
const constructorName = response.body && typeof response.body === 'object' ? (response.body as any).constructor?.name : 'unknown';
274-
logger.info(`Response body type: ${bodyType}, constructor: ${constructorName}`);
275-
}
276-
277257
// Send the response
278258
try {
279-
logger.info(`About to send response with status ${response.statusCode}`);
280-
res.status(response.statusCode);
281-
logger.info('Status set successfully');
282-
res.set(responseHeaders);
283-
logger.info('Headers set successfully');
284-
285-
// Handle any response errors
286-
res.on('error', err => {
287-
logger.error('Error sending response:', err);
288-
});
289-
290-
// Handle response finish event
291-
res.on('finish', () => {
292-
logger.info(`Response finished for ${compilerid}`);
293-
});
294-
295-
// Handle response close event
296-
res.on('close', () => {
297-
logger.info(`Response connection closed for ${compilerid}`);
298-
});
299-
300-
res.send(bodyBuffer);
301-
logger.info(`Called res.send for ${compilerid} with status ${response.statusCode}`);
259+
res.status(response.statusCode).set(responseHeaders).send(bodyBuffer);
260+
logger.info(`Successfully forwarded response for ${compilerid} with status ${response.statusCode}`);
302261
} catch (sendError) {
303262
logger.error(`Error sending response to client: ${(sendError as Error).message}`);
304-
logger.error('Send error stack:', (sendError as Error).stack);
305263
throw sendError; // Re-throw to be caught by outer catch
306264
}
307265
} catch (error) {
308266
logger.error('URL forwarding error:', error);
309-
logger.error('Error stack:', (error as Error).stack);
310-
logger.error('Error occurred after receiving response, might be a send issue');
311267

312268
// Check if response was already sent
313269
if (res.headersSent) {
@@ -316,7 +272,6 @@ export class CompilerExplorerRouter {
316272
}
317273

318274
const errorResponse = createErrorResponse(502, `Failed to forward request: ${(error as Error).message}`);
319-
logger.info(`Sending error response with status ${errorResponse.statusCode}`);
320275
res.status(errorResponse.statusCode).set(errorResponse.headers).send(errorResponse.body);
321276
}
322277
}

src/services/http-forwarder.ts

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ export function prepareForwardHeaders(headers: Record<string, string | string[]>
2121
}
2222
});
2323

24-
// Remove hop-by-hop headers
24+
// Remove hop-by-hop headers and headers that conflict with our processing
2525
delete forwardHeaders['connection'];
2626
delete forwardHeaders['upgrade'];
2727
delete forwardHeaders['proxy-authenticate'];
2828
delete forwardHeaders['proxy-authorization'];
2929
delete forwardHeaders['te'];
3030
delete forwardHeaders['trailers'];
31-
delete forwardHeaders['transfer-encoding'];
31+
delete forwardHeaders['transfer-encoding']; // Conflicts with content-length we set
3232

3333
return forwardHeaders;
3434
}
@@ -40,19 +40,17 @@ export async function forwardToEnvironmentUrl(
4040
isCmake: boolean,
4141
headers: Record<string, string | string[]>,
4242
): Promise<ForwardResponse> {
43-
const startTime = Date.now();
4443
try {
4544
const fullUrl = buildForwardUrl(targetUrl);
4645
const endpoint = isCmake ? 'cmake' : 'compile';
4746

4847
logger.info(`Forwarding ${endpoint} request for ${compilerId} to: ${fullUrl}`);
4948

5049
const forwardHeaders = prepareForwardHeaders(headers);
51-
logger.info('Forward headers:', forwardHeaders);
50+
logger.debug('Forward headers:', forwardHeaders);
5251

5352
// Make the HTTP request
54-
logger.info(`Making POST request to ${fullUrl} with body length: ${body.length}`);
55-
const requestStartTime = Date.now();
53+
logger.debug(`Making POST request to ${fullUrl} with body length: ${body.length}`);
5654
const response = await axios({
5755
method: 'POST',
5856
url: fullUrl,
@@ -66,36 +64,19 @@ export async function forwardToEnvironmentUrl(
6664
transformResponse: [data => data], // Don't let axios parse the response
6765
});
6866

69-
const requestDuration = Date.now() - requestStartTime;
7067
const responseBody = response.data || '';
71-
logger.info(
72-
`Received response from ${fullUrl}: status=${response.status}, body length=${responseBody.length}, request took ${requestDuration}ms`,
68+
logger.debug(
69+
`Received response from ${fullUrl}: status=${response.status}, body length=${responseBody.length}`,
7370
);
74-
logger.info('Response headers:', response.headers);
7571

7672
const result = {
7773
statusCode: response.status,
7874
headers: response.headers as Record<string, string>,
7975
body: responseBody,
8076
};
81-
const totalDuration = Date.now() - startTime;
82-
logger.info(
83-
`Returning response with status ${result.statusCode} and body length ${result.body.length}, total time ${totalDuration}ms`,
84-
);
8577
return result;
8678
} catch (error) {
8779
logger.error('HTTP forwarding error:', error);
88-
logger.error('Error details:', {
89-
message: (error as Error).message,
90-
code: axios.isAxiosError(error) ? error.code : undefined,
91-
response:
92-
axios.isAxiosError(error) && error.response
93-
? {
94-
status: error.response.status,
95-
data: error.response.data,
96-
}
97-
: undefined,
98-
});
9980

10081
if (axios.isAxiosError(error)) {
10182
if (error.code === 'ECONNABORTED') {

test/unit/http-forwarder.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,5 +97,19 @@ describe('HTTP Forwarder', () => {
9797
cookie: 'session=abc, preference=dark',
9898
});
9999
});
100+
101+
it('should remove transfer-encoding header to prevent conflicts with content-length', () => {
102+
const headers = {
103+
'content-type': 'application/json',
104+
'transfer-encoding': 'chunked',
105+
'content-length': '1234',
106+
'x-custom-header': 'value',
107+
};
108+
const result = prepareForwardHeaders(headers);
109+
expect(result['transfer-encoding']).toBeUndefined();
110+
expect(result['content-type']).toBe('application/json');
111+
expect(result['content-length']).toBe('1234');
112+
expect(result['x-custom-header']).toBe('value');
113+
});
100114
});
101115
});

0 commit comments

Comments
 (0)