Skip to content

Commit 6774a1a

Browse files
Andrew-PaystackAndrew-Paystack
authored
Merge pull request #18 from PaystackOSS/copilot/add-graceful-failure-response
Fix unhandled JSON parse error on non-JSON HTTP responses with MCP-compliant error handling
2 parents d462c99 + f0af09a commit 6774a1a

5 files changed

Lines changed: 297 additions & 7 deletions

File tree

‎src/paystack-client.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,15 @@ class PaystackClient {
6868
try {
6969
responseData = JSON.parse(responseText);
7070
} catch (parseError) {
71-
throw new Error(`Invalid JSON response: ${responseText}`);
71+
// Handle non-JSON responses gracefully (e.g., HTML error pages from API gateways)
72+
const responseSnippet = responseText.length > 200
73+
? responseText.substring(0, 200) + '...'
74+
: responseText;
75+
const errorMessage = `Received non-JSON response from server (HTTP ${response.status}): ${responseSnippet}`;
76+
const nonJsonError = new Error(errorMessage);
77+
(nonJsonError as any).statusCode = response.status;
78+
(nonJsonError as any).responseText = responseText;
79+
throw nonJsonError;
7280
}
7381
return responseData as PaystackResponse<T>;
7482
} catch (error) {

‎src/tools/get-paystack-operation.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ export function registerGetPaystackOperationTool(
3434
type: "text",
3535
text: `Operation with ID ${operation_id} not found.`,
3636
},
37-
]
37+
],
38+
isError: true
3839
}
3940
}
4041

@@ -47,14 +48,16 @@ export function registerGetPaystackOperationTool(
4748
},
4849
]
4950
}
50-
} catch {
51+
} catch (error) {
52+
const errorMessage = error instanceof Error ? error.message : String(error);
5153
return {
5254
content: [
5355
{
5456
type: "text",
55-
text: `Operation with ID ${operation_id} not found.`,
57+
text: `Error retrieving operation: ${errorMessage}`,
5658
},
57-
]
59+
],
60+
isError: true
5861
}
5962
}
6063
}

‎src/tools/make-paystack-request.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,23 @@ export function registerMakePaystackRequestTool(server: McpServer) {
3838
]
3939
}
4040
} catch(error) {
41+
// Follow MCP best practices: return isError flag instead of throwing
42+
const errorMessage = error instanceof Error ? error.message : String(error);
43+
const statusCode = (error as any).statusCode;
44+
45+
let detailedMessage = `Unable to make request: ${errorMessage}`;
46+
if (statusCode) {
47+
detailedMessage = `Unable to make request (HTTP ${statusCode}): ${errorMessage}`;
48+
}
49+
4150
return {
4251
content: [
4352
{
4453
type: "text",
45-
text: `Unable to make request. ${error}`,
54+
text: detailedMessage,
4655
},
47-
]
56+
],
57+
isError: true
4858
}
4959
}
5060
}

‎test/make-paystack-request-tool.spec.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import assert from "node:assert";
2+
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
3+
import { registerMakePaystackRequestTool } from "../src/tools/make-paystack-request.js";
4+
5+
describe("MakePaystackRequestTool", () => {
6+
describe("Error handling with isError flag", () => {
7+
let server: McpServer;
8+
let toolHandler: any;
9+
10+
before(() => {
11+
// Create a mock MCP server
12+
server = {
13+
registerTool: (name: string, config: any, handler: any) => {
14+
if (name === "make_paystack_request") {
15+
toolHandler = handler;
16+
}
17+
}
18+
} as any;
19+
20+
registerMakePaystackRequestTool(server);
21+
});
22+
23+
it("should return isError: true for non-JSON responses", async () => {
24+
// Mock fetch to return HTML error page
25+
const originalFetch = global.fetch;
26+
global.fetch = async () => {
27+
return {
28+
status: 502,
29+
text: async () => "<html><body><h1>502 Bad Gateway</h1></body></html>",
30+
} as Response;
31+
};
32+
33+
try {
34+
const result = await toolHandler({
35+
request: {
36+
method: "GET",
37+
path: "/test-endpoint",
38+
}
39+
});
40+
41+
// Verify isError flag is set
42+
assert.strictEqual(result.isError, true);
43+
44+
// Verify error message content
45+
assert.ok(result.content);
46+
assert.strictEqual(result.content.length, 1);
47+
assert.strictEqual(result.content[0].type, "text");
48+
assert.ok(result.content[0].text.includes("Unable to make request"));
49+
assert.ok(result.content[0].text.includes("HTTP 502"));
50+
assert.ok(result.content[0].text.includes("non-JSON response"));
51+
} finally {
52+
global.fetch = originalFetch;
53+
}
54+
});
55+
56+
it("should omit isError for successful responses", async () => {
57+
// Mock fetch to return valid JSON
58+
const originalFetch = global.fetch;
59+
const validJsonResponse = {
60+
status: true,
61+
message: "Success",
62+
data: { id: 123 }
63+
};
64+
65+
global.fetch = async () => {
66+
return {
67+
status: 200,
68+
text: async () => JSON.stringify(validJsonResponse),
69+
} as Response;
70+
};
71+
72+
try {
73+
const result = await toolHandler({
74+
request: {
75+
method: "GET",
76+
path: "/test-endpoint",
77+
}
78+
});
79+
80+
// Verify isError is not set (or false) for successful responses
81+
assert.ok(!result.isError);
82+
83+
// Verify success content
84+
assert.ok(result.content);
85+
assert.strictEqual(result.content.length, 1);
86+
assert.strictEqual(result.content[0].type, "text");
87+
assert.strictEqual(result.content[0].mimeType, "application/json");
88+
89+
// Parse and verify the response data
90+
const parsedResponse = JSON.parse(result.content[0].text);
91+
assert.strictEqual(parsedResponse.status, true);
92+
assert.strictEqual(parsedResponse.message, "Success");
93+
} finally {
94+
global.fetch = originalFetch;
95+
}
96+
});
97+
98+
it("should include HTTP status code in error message", async () => {
99+
// Mock fetch to return a 504 Gateway Timeout
100+
const originalFetch = global.fetch;
101+
global.fetch = async () => {
102+
return {
103+
status: 504,
104+
text: async () => "Gateway Timeout",
105+
} as Response;
106+
};
107+
108+
try {
109+
const result = await toolHandler({
110+
request: {
111+
method: "POST",
112+
path: "/transaction/initialize",
113+
data: { amount: 1000 }
114+
}
115+
});
116+
117+
// Verify error response structure
118+
assert.strictEqual(result.isError, true);
119+
assert.ok(result.content[0].text.includes("HTTP 504"));
120+
} finally {
121+
global.fetch = originalFetch;
122+
}
123+
});
124+
125+
it("should handle network errors with isError flag", async () => {
126+
// Mock fetch to simulate network error
127+
const originalFetch = global.fetch;
128+
global.fetch = async () => {
129+
throw new Error("Network connection failed");
130+
};
131+
132+
try {
133+
const result = await toolHandler({
134+
request: {
135+
method: "GET",
136+
path: "/customer/list",
137+
}
138+
});
139+
140+
// Verify error is properly handled
141+
assert.strictEqual(result.isError, true);
142+
assert.ok(result.content[0].text.includes("Unable to make request"));
143+
} finally {
144+
global.fetch = originalFetch;
145+
}
146+
});
147+
});
148+
});

‎test/paystack-client.spec.ts‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import assert from "node:assert";
2+
import { paystackClient } from "../src/paystack-client.js";
3+
4+
describe("PaystackClient", () => {
5+
describe("makeRequest - Non-JSON Response Handling", () => {
6+
it("should throw a descriptive error for HTML error responses", async () => {
7+
// This test validates that non-JSON responses (like HTML error pages)
8+
// are handled gracefully with proper error messages including status code
9+
10+
// Mock fetch to return an HTML 502 Bad Gateway response
11+
const originalFetch = global.fetch;
12+
global.fetch = async () => {
13+
return {
14+
status: 502,
15+
text: async () => "<html><body><h1>502 Bad Gateway</h1></body></html>",
16+
} as Response;
17+
};
18+
19+
try {
20+
await paystackClient.makeRequest("GET", "/test-endpoint");
21+
assert.fail("Expected makeRequest to throw an error");
22+
} catch (error: any) {
23+
// Verify error message includes status code and response snippet
24+
assert.ok(error.message.includes("Received non-JSON response from server"));
25+
assert.ok(error.message.includes("HTTP 502"));
26+
assert.ok(error.message.includes("<html>"));
27+
28+
// Verify statusCode is attached to error
29+
assert.strictEqual(error.statusCode, 502);
30+
31+
// Verify full responseText is available for debugging
32+
assert.ok(error.responseText);
33+
assert.ok(error.responseText.includes("502 Bad Gateway"));
34+
} finally {
35+
global.fetch = originalFetch;
36+
}
37+
});
38+
39+
it("should truncate long non-JSON responses to 200 characters", async () => {
40+
const originalFetch = global.fetch;
41+
const longHtmlResponse = "<html>" + "x".repeat(300) + "</html>";
42+
43+
global.fetch = async () => {
44+
return {
45+
status: 500,
46+
text: async () => longHtmlResponse,
47+
} as Response;
48+
};
49+
50+
try {
51+
await paystackClient.makeRequest("GET", "/test-endpoint");
52+
assert.fail("Expected makeRequest to throw an error");
53+
} catch (error: any) {
54+
// Verify the error message contains truncated snippet (200 chars + '...')
55+
const snippetMatch = error.message.match(/: (.+)$/);
56+
assert.ok(snippetMatch);
57+
const snippet = snippetMatch[1];
58+
59+
// Should end with '...' for truncation
60+
assert.ok(snippet.endsWith('...'));
61+
62+
// Should be 203 characters (200 + '...')
63+
assert.ok(snippet.length <= 203);
64+
65+
// Full response should still be available
66+
assert.strictEqual(error.responseText, longHtmlResponse);
67+
} finally {
68+
global.fetch = originalFetch;
69+
}
70+
});
71+
72+
it("should not truncate short non-JSON responses", async () => {
73+
const originalFetch = global.fetch;
74+
const shortResponse = "Gateway Timeout";
75+
76+
global.fetch = async () => {
77+
return {
78+
status: 504,
79+
text: async () => shortResponse,
80+
} as Response;
81+
};
82+
83+
try {
84+
await paystackClient.makeRequest("GET", "/test-endpoint");
85+
assert.fail("Expected makeRequest to throw an error");
86+
} catch (error: any) {
87+
// Verify the error message contains full short response
88+
assert.ok(error.message.includes(shortResponse));
89+
assert.ok(!error.message.includes('...'));
90+
assert.strictEqual(error.statusCode, 504);
91+
} finally {
92+
global.fetch = originalFetch;
93+
}
94+
});
95+
96+
it("should successfully parse valid JSON responses", async () => {
97+
const originalFetch = global.fetch;
98+
const validJsonResponse = {
99+
status: true,
100+
message: "Success",
101+
data: { id: 123 }
102+
};
103+
104+
global.fetch = async () => {
105+
return {
106+
status: 200,
107+
text: async () => JSON.stringify(validJsonResponse),
108+
} as Response;
109+
};
110+
111+
try {
112+
const response = await paystackClient.makeRequest("GET", "/test-endpoint");
113+
assert.strictEqual(response.status, true);
114+
assert.strictEqual(response.message, "Success");
115+
assert.deepStrictEqual(response.data, { id: 123 });
116+
} finally {
117+
global.fetch = originalFetch;
118+
}
119+
});
120+
});
121+
});

0 commit comments

Comments
 (0)