Skip to content

Commit 3b911fa

Browse files
authored
Refactor api-proxy upstream response factory into focused modules (#5445)
* Initial plan * refactor(api-proxy): split upstream handlers --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 10ab27e commit 3b911fa

8 files changed

Lines changed: 411 additions & 197 deletions

File tree

containers/api-proxy/Dockerfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ COPY server.js logging.js metrics.js rate-limiter.js \
2727
ai-credits-pricing.js models-dev-catalog.js models.dev.catalog.json \
2828
oidc-refresh-utils.js body-transform.js body-utils.js rate-limit.js websocket-proxy.js \
2929
deprecated-header-tracker.js billing-headers.js upstream-response.js \
30+
upstream-log.js upstream-retry.js upstream-token.js \
3031
anthropic-cache.js otel.js otel-exporters.js otel-serialization.js \
3132
token-budget-log.js blocked-request-diagnostics.js \
3233
provider-env-constants.js provider-names.js ./
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
'use strict';
2+
3+
const { COPILOT_PLACEHOLDER_TOKEN } = require('./providers/copilot-byok');
4+
const { stripBearerPrefix } = require('./providers/copilot-auth');
5+
6+
function buildCopilotAuthErrorMessage(statusCode, env = process.env) {
7+
const baseMessage = `Upstream returned ${statusCode}`;
8+
const byokBaseUrl = (env.COPILOT_PROVIDER_BASE_URL || '').trim();
9+
const byokKey = stripBearerPrefix(env.COPILOT_PROVIDER_API_KEY);
10+
const hasByokBaseUrl = Boolean(byokBaseUrl);
11+
12+
if (hasByokBaseUrl && byokKey === COPILOT_PLACEHOLDER_TOKEN) {
13+
return `${baseMessage} — COPILOT_PROVIDER_API_KEY is the AWF placeholder sentinel. ` +
14+
'This indicates an internal credential-isolation misconfiguration (real BYOK key not forwarded to api-proxy).';
15+
}
16+
17+
if (hasByokBaseUrl && !byokKey) {
18+
return `${baseMessage} — BYOK provider request to COPILOT_PROVIDER_BASE_URL failed because COPILOT_PROVIDER_API_KEY is not set.`;
19+
}
20+
21+
if (hasByokBaseUrl) {
22+
return `${baseMessage} — BYOK provider request to COPILOT_PROVIDER_BASE_URL failed. ` +
23+
'Verify COPILOT_PROVIDER_BASE_URL and COPILOT_PROVIDER_API_KEY.';
24+
}
25+
26+
return `${baseMessage} — check that the API key is valid and correctly formatted`;
27+
}
28+
29+
function createLogRequestCompletion({ metrics, logRequest, sanitizeForLog, applyMaxRunsInvocation }) {
30+
return function logRequestCompletion(statusCode, responseBytes, initiatorSent, billingInfo, {
31+
startTime, provider, req, requestBytes, targetHost, requestId,
32+
}) {
33+
const duration = Date.now() - startTime;
34+
const sc = metrics.statusClass(statusCode);
35+
metrics.gaugeDec('active_requests', { provider });
36+
metrics.increment('requests_total', { provider, method: req.method, status_class: sc });
37+
metrics.increment('response_bytes_total', { provider }, responseBytes);
38+
metrics.observe('request_duration_ms', duration, { provider });
39+
if (statusCode >= 200 && statusCode < 300) {
40+
applyMaxRunsInvocation();
41+
}
42+
const logFields = {
43+
request_id: requestId, provider, method: req.method,
44+
path: sanitizeForLog(req.url), status: statusCode,
45+
duration_ms: duration, request_bytes: requestBytes,
46+
response_bytes: responseBytes, upstream_host: targetHost,
47+
};
48+
if (initiatorSent) logFields.x_initiator = initiatorSent;
49+
if (billingInfo) logFields.billing = billingInfo;
50+
logRequest('info', 'request_complete', logFields);
51+
};
52+
}
53+
54+
function createLogUpstreamAuthError({
55+
logRequest,
56+
sanitizeForLog,
57+
applyPermissionDenied,
58+
parseModelNotSupportedFromBody,
59+
}) {
60+
return function logUpstreamAuthError(statusCode, { requestId, provider, targetHost, req, responseBody }) {
61+
const authErrorMessage = provider === 'copilot'
62+
? buildCopilotAuthErrorMessage(statusCode)
63+
: `Upstream returned ${statusCode} — check that the API key is valid and correctly formatted`;
64+
65+
if (statusCode === 401 || statusCode === 403) {
66+
applyPermissionDenied();
67+
logRequest('warn', 'upstream_auth_error', {
68+
request_id: requestId, provider, status: statusCode,
69+
upstream_host: targetHost, path: sanitizeForLog(req.url),
70+
message: authErrorMessage,
71+
});
72+
} else if (statusCode === 400) {
73+
// Suppress generic auth-error message when the 400 is a model-not-supported
74+
// error — that case is handled by the model_unavailable diagnostic.
75+
if (responseBody && parseModelNotSupportedFromBody(responseBody)) return;
76+
logRequest('warn', 'upstream_auth_error', {
77+
request_id: requestId, provider, status: statusCode,
78+
upstream_host: targetHost, path: sanitizeForLog(req.url),
79+
message: authErrorMessage,
80+
});
81+
}
82+
};
83+
}
84+
85+
module.exports = {
86+
createLogRequestCompletion,
87+
createLogUpstreamAuthError,
88+
buildCopilotAuthErrorMessage,
89+
};
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
const {
2+
createLogRequestCompletion,
3+
createLogUpstreamAuthError,
4+
} = require('./upstream-log');
5+
6+
describe('upstream-log', () => {
7+
test('logRequestCompletion records metrics and invokes max-runs on success', () => {
8+
const metrics = {
9+
statusClass: jest.fn(() => '2xx'),
10+
gaugeDec: jest.fn(),
11+
increment: jest.fn(),
12+
observe: jest.fn(),
13+
};
14+
const logRequest = jest.fn();
15+
const applyMaxRunsInvocation = jest.fn();
16+
const logRequestCompletion = createLogRequestCompletion({
17+
metrics,
18+
logRequest,
19+
sanitizeForLog: (value) => value,
20+
applyMaxRunsInvocation,
21+
});
22+
23+
logRequestCompletion(200, 42, 'agent', { prompt_tokens: 10 }, {
24+
startTime: Date.now() - 5,
25+
provider: 'copilot',
26+
req: { method: 'POST', url: '/v1/chat/completions' },
27+
requestBytes: 12,
28+
targetHost: 'api.githubcopilot.com',
29+
requestId: 'req-1',
30+
});
31+
32+
expect(metrics.gaugeDec).toHaveBeenCalledWith('active_requests', { provider: 'copilot' });
33+
expect(applyMaxRunsInvocation).toHaveBeenCalledTimes(1);
34+
expect(logRequest).toHaveBeenCalledWith('info', 'request_complete', expect.objectContaining({
35+
request_id: 'req-1',
36+
status: 200,
37+
x_initiator: 'agent',
38+
}));
39+
});
40+
41+
test('logUpstreamAuthError suppresses 400 model-not-supported auth log noise', () => {
42+
const logRequest = jest.fn();
43+
const applyPermissionDenied = jest.fn();
44+
const logUpstreamAuthError = createLogUpstreamAuthError({
45+
logRequest,
46+
sanitizeForLog: (value) => value,
47+
applyPermissionDenied,
48+
parseModelNotSupportedFromBody: () => true,
49+
});
50+
51+
logUpstreamAuthError(400, {
52+
requestId: 'req-1',
53+
provider: 'copilot',
54+
targetHost: 'api.githubcopilot.com',
55+
req: { url: '/v1/chat/completions' },
56+
responseBody: Buffer.from('The requested model is not supported'),
57+
});
58+
59+
expect(logRequest).not.toHaveBeenCalled();
60+
expect(applyPermissionDenied).not.toHaveBeenCalled();
61+
});
62+
});

0 commit comments

Comments
 (0)