Skip to content

Commit bbdd582

Browse files
committed
fix(oauth): enhance error handling in proxy client
- Provide user-friendly error messages based on HTTP status codes - Fix middleware placement for JSON body parsing - Update tests for new error message format
1 parent 496e3c5 commit bbdd582

File tree

3 files changed

+61
-12
lines changed

3 files changed

+61
-12
lines changed

client/src/lib/__tests__/oauth-proxy.test.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ describe("OAuth Proxy Utilities", () => {
109109
it("should handle non-ok responses", async () => {
110110
(global.fetch as jest.Mock).mockResolvedValueOnce({
111111
ok: false,
112+
status: 404,
112113
statusText: "Not Found",
113114
json: async () => ({ error: "Metadata not found" }),
114115
});
@@ -119,13 +120,14 @@ describe("OAuth Proxy Utilities", () => {
119120
mockConfig,
120121
),
121122
).rejects.toThrow(
122-
"Failed to discover OAuth metadata: Metadata not found",
123+
"Failed to discover OAuth metadata: Not found: Metadata not found",
123124
);
124125
});
125126

126127
it("should handle responses without error details", async () => {
127128
(global.fetch as jest.Mock).mockResolvedValueOnce({
128129
ok: false,
130+
status: 500,
129131
statusText: "Internal Server Error",
130132
json: async () => {
131133
throw new Error("Invalid JSON");
@@ -138,7 +140,7 @@ describe("OAuth Proxy Utilities", () => {
138140
mockConfig,
139141
),
140142
).rejects.toThrow(
141-
"Failed to discover OAuth metadata: Internal Server Error",
143+
"Failed to discover OAuth metadata: Server error (500): Internal Server Error",
142144
);
143145
});
144146
});
@@ -177,6 +179,7 @@ describe("OAuth Proxy Utilities", () => {
177179
it("should handle errors", async () => {
178180
(global.fetch as jest.Mock).mockResolvedValueOnce({
179181
ok: false,
182+
status: 404,
180183
statusText: "Not Found",
181184
json: async () => ({ error: "Resource metadata not found" }),
182185
});
@@ -187,7 +190,7 @@ describe("OAuth Proxy Utilities", () => {
187190
mockConfig,
188191
),
189192
).rejects.toThrow(
190-
"Failed to discover resource metadata: Resource metadata not found",
193+
"Failed to discover resource metadata: Not found: Resource metadata not found",
191194
);
192195
});
193196
});
@@ -237,6 +240,7 @@ describe("OAuth Proxy Utilities", () => {
237240
it("should handle registration errors", async () => {
238241
(global.fetch as jest.Mock).mockResolvedValueOnce({
239242
ok: false,
243+
status: 400,
240244
statusText: "Bad Request",
241245
json: async () => ({ error: "Invalid client metadata" }),
242246
});
@@ -250,7 +254,9 @@ describe("OAuth Proxy Utilities", () => {
250254
},
251255
mockConfig,
252256
),
253-
).rejects.toThrow("Failed to register client: Invalid client metadata");
257+
).rejects.toThrow(
258+
"Failed to register client: Bad Request: Invalid client metadata",
259+
);
254260
});
255261
});
256262

@@ -301,6 +307,7 @@ describe("OAuth Proxy Utilities", () => {
301307
it("should handle token exchange errors", async () => {
302308
(global.fetch as jest.Mock).mockResolvedValueOnce({
303309
ok: false,
310+
status: 401,
304311
statusText: "Unauthorized",
305312
json: async () => ({ error: "invalid_grant" }),
306313
});
@@ -311,7 +318,9 @@ describe("OAuth Proxy Utilities", () => {
311318
{ grant_type: "authorization_code", code: "invalid" },
312319
mockConfig,
313320
),
314-
).rejects.toThrow("Failed to exchange authorization code: invalid_grant");
321+
).rejects.toThrow(
322+
"Failed to exchange authorization code: Authentication failed: invalid_grant",
323+
);
315324
});
316325

317326
it("should handle network failures", async () => {

client/src/lib/oauth-proxy.ts

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,56 @@ async function proxyFetch<T>(
5959
});
6060
}
6161

62-
const response = await fetch(url.toString(), {
63-
method,
64-
headers: getProxyHeaders(config),
65-
...(body && { body: JSON.stringify(body) }),
66-
});
62+
let response: Response;
63+
try {
64+
response = await fetch(url.toString(), {
65+
method,
66+
headers: getProxyHeaders(config),
67+
...(body && { body: JSON.stringify(body) }),
68+
});
69+
} catch (error) {
70+
// Network errors (connection refused, timeout, etc.)
71+
throw new Error(
72+
`Network error: ${error instanceof Error ? error.message : "Failed to connect to proxy server"}`,
73+
);
74+
}
6775

6876
if (!response.ok) {
69-
const error = await response
77+
// Try to get detailed error from response body
78+
const errorData = await response
7079
.json()
7180
.catch(() => ({ error: response.statusText }));
72-
throw new Error(error.error || response.statusText);
81+
const errorMessage = errorData.error || response.statusText;
82+
const errorDetails = errorData.details ? ` - ${errorData.details}` : "";
83+
84+
// Provide specific error messages based on status code
85+
switch (response.status) {
86+
case 400:
87+
throw new Error(`Bad Request: ${errorMessage}${errorDetails}`);
88+
case 401:
89+
throw new Error(
90+
`Authentication failed: ${errorMessage}. Check your proxy authentication token.`,
91+
);
92+
case 403:
93+
throw new Error(
94+
`Access forbidden: ${errorMessage}. You may not have permission to access this resource.`,
95+
);
96+
case 404:
97+
throw new Error(
98+
`Not found: ${errorMessage}. The OAuth endpoint may not exist or be misconfigured.`,
99+
);
100+
case 500:
101+
case 502:
102+
case 503:
103+
case 504:
104+
throw new Error(
105+
`Server error (${response.status}): ${errorMessage}${errorDetails}`,
106+
);
107+
default:
108+
throw new Error(
109+
`Request failed (${response.status}): ${errorMessage}${errorDetails}`,
110+
);
111+
}
73112
}
74113

75114
return await response.json();

server/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ const updateHeadersInPlace = (
165165

166166
const app = express();
167167
app.use(cors());
168+
app.use(express.json()); // Enable JSON body parsing globally
168169
app.use((req, res, next) => {
169170
res.header("Access-Control-Expose-Headers", "mcp-session-id");
170171
next();

0 commit comments

Comments
 (0)