Skip to content

Commit a5eddb9

Browse files
vincentkocvincentkoc
committed
fix(msteams): bound service error bodies
1 parent 56302f7 commit a5eddb9

11 files changed

Lines changed: 167 additions & 51 deletions

‎extensions/msteams/src/graph-upload.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ function expectGraphUploadFetch(fetchFn: ReturnType<typeof vi.fn>, expectedUrl:
2222
expect(init?.headers?.["User-Agent"]).toMatch(/^teams\.ts\[apps\]\/.+ OpenClaw\/.+$/);
2323
}
2424

25+
function bodyOnlyErrorResponse(body: string, status = 500): Response {
26+
return {
27+
ok: false,
28+
status,
29+
headers: new Headers(),
30+
body: new Response(body).body,
31+
} as unknown as Response;
32+
}
33+
2534
describe("graph upload helpers", () => {
2635
const tokenProvider = {
2736
getAccessToken: vi.fn(async () => "graph-token"),
@@ -107,6 +116,31 @@ describe("graph upload helpers", () => {
107116
}),
108117
).rejects.toThrow("SharePoint upload response missing required fields");
109118
});
119+
120+
it("bounds upload error bodies without requiring response.text()", async () => {
121+
const fetchFn = vi.fn(async () =>
122+
bodyOnlyErrorResponse(`${"upload-denied ".repeat(4096)}tail-marker`, 413),
123+
);
124+
125+
let error: unknown;
126+
try {
127+
await uploadToSharePoint({
128+
buffer: Buffer.from("world"),
129+
filename: "large.txt",
130+
siteId: "site-123",
131+
tokenProvider,
132+
fetchFn: fetchFn as unknown as typeof fetch,
133+
});
134+
} catch (caught) {
135+
error = caught;
136+
}
137+
138+
expect(error).toBeInstanceOf(Error);
139+
const message = (error as Error).message;
140+
expect(message).toContain("SharePoint upload failed (413): upload-denied");
141+
expect(message).not.toContain("tail-marker");
142+
expect(message.length).toBeLessThan(700);
143+
});
110144
});
111145

112146
describe("resolveGraphChatId", () => {

‎extensions/msteams/src/graph-upload.ts‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*/
1111

1212
import type { MSTeamsAccessTokenProvider } from "./attachments/types.js";
13+
import { createMSTeamsHttpError } from "./http-error.js";
1314
import { buildUserAgent } from "./user-agent.js";
1415

1516
const GRAPH_ROOT = "https://graph.microsoft.com/v1.0";
@@ -50,8 +51,7 @@ export async function uploadToOneDrive(params: {
5051
});
5152

5253
if (!res.ok) {
53-
const body = await res.text().catch(() => "");
54-
throw new Error(`OneDrive upload failed: ${res.status} ${res.statusText} - ${body}`);
54+
throw await createMSTeamsHttpError(res, "OneDrive upload failed");
5555
}
5656

5757
const data = (await res.json()) as {
@@ -103,8 +103,7 @@ async function createSharingLink(params: {
103103
});
104104

105105
if (!res.ok) {
106-
const body = await res.text().catch(() => "");
107-
throw new Error(`Create sharing link failed: ${res.status} ${res.statusText} - ${body}`);
106+
throw await createMSTeamsHttpError(res, "Create sharing link failed");
108107
}
109108

110109
const data = (await res.json()) as {
@@ -198,8 +197,7 @@ export async function uploadToSharePoint(params: {
198197
);
199198

200199
if (!res.ok) {
201-
const body = await res.text().catch(() => "");
202-
throw new Error(`SharePoint upload failed: ${res.status} ${res.statusText} - ${body}`);
200+
throw await createMSTeamsHttpError(res, "SharePoint upload failed");
203201
}
204202

205203
const data = (await res.json()) as {
@@ -259,8 +257,7 @@ export async function getDriveItemProperties(params: {
259257
);
260258

261259
if (!res.ok) {
262-
const body = await res.text().catch(() => "");
263-
throw new Error(`Get driveItem properties failed: ${res.status} ${res.statusText} - ${body}`);
260+
throw await createMSTeamsHttpError(res, "Get driveItem properties failed");
264261
}
265262

266263
const data = (await res.json()) as {
@@ -371,8 +368,7 @@ async function getChatMembers(params: {
371368
});
372369

373370
if (!res.ok) {
374-
const body = await res.text().catch(() => "");
375-
throw new Error(`Get chat members failed: ${res.status} ${res.statusText} - ${body}`);
371+
throw await createMSTeamsHttpError(res, "Get chat members failed");
376372
}
377373

378374
const data = (await res.json()) as {
@@ -436,10 +432,7 @@ async function createSharePointSharingLink(params: {
436432
);
437433

438434
if (!res.ok) {
439-
const respBody = await res.text().catch(() => "");
440-
throw new Error(
441-
`Create SharePoint sharing link failed: ${res.status} ${res.statusText} - ${respBody}`,
442-
);
435+
throw await createMSTeamsHttpError(res, "Create SharePoint sharing link failed");
443436
}
444437

445438
const data = (await res.json()) as {

‎extensions/msteams/src/graph.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { GRAPH_ROOT } from "./attachments/shared.js";
55
const GRAPH_BETA = "https://graph.microsoft.com/beta";
66
const NULL_BODY_STATUSES = new Set([101, 204, 205, 304]);
77
import { createMSTeamsTokenProvider, loadMSTeamsSdkWithAuth } from "./sdk.js";
8+
import { createMSTeamsHttpError } from "./http-error.js";
89
import { readAccessToken } from "./token-response.js";
910
import { resolveDelegatedAccessToken, resolveMSTeamsCredentials } from "./token.js";
1011
import { buildUserAgent } from "./user-agent.js";
@@ -65,9 +66,9 @@ async function requestGraph(params: {
6566
});
6667
try {
6768
if (!response.ok) {
68-
const text = await response.text().catch(() => "");
69-
throw new Error(
70-
`${params.errorPrefix ?? "Graph"} ${params.path} failed (${response.status}): ${text || "unknown error"}`,
69+
throw await createMSTeamsHttpError(
70+
response,
71+
`${params.errorPrefix ?? "Graph"} ${params.path} failed`,
7172
);
7273
}
7374
const body = NULL_BODY_STATUSES.has(response.status) ? null : await response.arrayBuffer();
@@ -131,10 +132,7 @@ export async function fetchGraphAbsoluteUrl<T>(params: {
131132
});
132133
try {
133134
if (!response.ok) {
134-
const text = await response.text().catch(() => "");
135-
throw new Error(
136-
`Graph ${params.url} failed (${response.status}): ${text || "unknown error"}`,
137-
);
135+
throw await createMSTeamsHttpError(response, `Graph ${params.url} failed`);
138136
}
139137
return await readProviderJsonResponse<T>(response, `Graph ${params.url} failed`);
140138
} finally {

‎extensions/msteams/src/http-error.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { describe, expect, it } from "vitest";
2+
import { createMSTeamsHttpError, readMSTeamsHttpErrorDetail } from "./http-error.js";
3+
4+
function bodyOnlyErrorResponse(body: string, status = 429): Response {
5+
return {
6+
ok: false,
7+
status,
8+
headers: new Headers(),
9+
body: new Response(body).body,
10+
} as unknown as Response;
11+
}
12+
13+
describe("msteams http errors", () => {
14+
it("creates bounded provider errors without relying on response.text()", async () => {
15+
const error = await createMSTeamsHttpError(
16+
bodyOnlyErrorResponse(`${"x".repeat(24 * 1024)}tail-marker`),
17+
"Teams request failed",
18+
);
19+
20+
expect(error.message).toContain("Teams request failed (429):");
21+
expect(error.message).not.toContain("tail-marker");
22+
expect(error.message.length).toBeLessThan(700);
23+
expect((error as { statusCode?: number }).statusCode).toBe(429);
24+
});
25+
26+
it("returns a bounded response detail for non-throwing callers", async () => {
27+
const detail = await readMSTeamsHttpErrorDetail(
28+
bodyOnlyErrorResponse(`${"denied ".repeat(4096)}tail-marker`, 403),
29+
"HTTP 403",
30+
);
31+
32+
expect(detail).toContain("denied");
33+
expect(detail).not.toContain("tail-marker");
34+
expect(detail.length).toBeLessThan(700);
35+
});
36+
});

‎extensions/msteams/src/http-error.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import {
2+
createProviderHttpError,
3+
extractProviderErrorDetail,
4+
} from "openclaw/plugin-sdk/provider-http";
5+
6+
export async function createMSTeamsHttpError(
7+
response: Response,
8+
label: string,
9+
options?: { statusPrefix?: string },
10+
): Promise<Error> {
11+
return await createProviderHttpError(response, label, options);
12+
}
13+
14+
export async function readMSTeamsHttpErrorDetail(
15+
response: Response,
16+
fallback: string,
17+
): Promise<string> {
18+
return (await extractProviderErrorDetail(response).catch(() => undefined)) ?? fallback;
19+
}

‎extensions/msteams/src/monitor-handler.sso.test.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,12 @@ function createFakeFetch(handlers: Array<(url: string, init?: unknown) => unknow
127127
status: number;
128128
body: unknown;
129129
};
130-
return {
131-
ok: response.ok,
130+
const responseBody = response.body;
131+
const isTextBody = typeof responseBody === "string";
132+
return new Response(isTextBody ? responseBody : JSON.stringify(responseBody ?? ""), {
132133
status: response.status,
133-
json: async () => response.body,
134-
text: async () =>
135-
typeof response.body === "string" ? response.body : JSON.stringify(response.body ?? ""),
136-
};
134+
headers: { "content-type": isTextBody ? "text/plain" : "application/json" },
135+
});
137136
};
138137
return { fetchImpl, calls };
139138
}

‎extensions/msteams/src/oauth.token.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
buildMSTeamsTokenEndpoint,
88
type MSTeamsDelegatedTokens,
99
} from "./oauth.shared.js";
10+
import { createMSTeamsHttpError } from "./http-error.js";
1011

1112
/** Five-minute buffer subtracted from token expiry to avoid edge-case clock drift. */
1213
const EXPIRY_BUFFER_MS = 5 * 60 * 1000;
@@ -63,8 +64,7 @@ async function fetchMSTeamsTokens(params: {
6364

6465
try {
6566
if (!response.ok) {
66-
const errorText = await response.text();
67-
throw new Error(`MSTeams ${params.failureLabel} failed (${response.status}): ${errorText}`);
67+
throw await createMSTeamsHttpError(response, `MSTeams ${params.failureLabel} failed`);
6868
}
6969
return await readProviderJsonResponse<MSTeamsTokenResponse>(
7070
response,

‎extensions/msteams/src/sdk.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
tryNormalizeBotFrameworkServiceUrl,
1111
} from "./bot-framework-service-url.js";
1212
import { formatUnknownError } from "./errors.js";
13+
import { createMSTeamsHttpError } from "./http-error.js";
1314
import type { MSTeamsAdapter } from "./messenger.js";
1415
import type { MSTeamsCredentials, MSTeamsFederatedCredentials } from "./token.js";
1516
import { buildUserAgent } from "./user-agent.js";
@@ -491,9 +492,8 @@ async function updateActivityViaRest(params: {
491492

492493
try {
493494
if (!response.ok) {
494-
const body = await response.text().catch(() => "");
495-
throw Object.assign(new Error(`updateActivity failed: HTTP ${response.status} ${body}`), {
496-
statusCode: response.status,
495+
throw await createMSTeamsHttpError(response, "updateActivity failed", {
496+
statusPrefix: "HTTP ",
497497
});
498498
}
499499

@@ -538,9 +538,8 @@ async function deleteActivityViaRest(params: {
538538

539539
try {
540540
if (!response.ok) {
541-
const body = await response.text().catch(() => "");
542-
throw Object.assign(new Error(`deleteActivity failed: HTTP ${response.status} ${body}`), {
543-
statusCode: response.status,
541+
throw await createMSTeamsHttpError(response, "deleteActivity failed", {
542+
statusPrefix: "HTTP ",
544543
});
545544
}
546545
} finally {

‎extensions/msteams/src/sso.ts‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
*/
2626

2727
import type { MSTeamsAccessTokenProvider } from "./attachments/types.js";
28+
import { readMSTeamsHttpErrorDetail } from "./http-error.js";
2829
import type { MSTeamsSsoTokenStore } from "./sso-token-store.js";
2930
import { buildUserAgent } from "./user-agent.js";
3031

@@ -49,17 +50,8 @@ type BotFrameworkUserTokenResponse = {
4950

5051
export type MSTeamsSsoFetch = (
5152
input: string,
52-
init?: {
53-
method?: string;
54-
headers?: Record<string, string>;
55-
body?: string;
56-
},
57-
) => Promise<{
58-
ok: boolean;
59-
status: number;
60-
json(): Promise<unknown>;
61-
text(): Promise<string>;
62-
}>;
53+
init?: RequestInit,
54+
) => Promise<Response>;
6355

6456
export type MSTeamsSsoDeps = {
6557
tokenProvider: MSTeamsAccessTokenProvider;
@@ -162,8 +154,8 @@ async function callUserTokenService(
162154
body: params.body === undefined ? undefined : JSON.stringify(params.body),
163155
});
164156
if (!response.ok) {
165-
const text = await response.text().catch(() => "");
166-
return { error: text || `HTTP ${response.status}`, status: response.status };
157+
const error = await readMSTeamsHttpErrorDetail(response, `HTTP ${response.status}`);
158+
return { error, status: response.status };
167159
}
168160
let parsed: unknown;
169161
try {

‎src/agents/provider-http-errors.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,45 @@ describe("provider error utils", () => {
3939
expect(extractProviderRequestId(response)).toBe("fallback_req");
4040
});
4141

42+
it("preserves OAuth error descriptions as actionable details", async () => {
43+
const response = new Response(
44+
JSON.stringify({
45+
error: "invalid_request",
46+
error_description: "AADSTS7000215: Invalid client secret provided.",
47+
}),
48+
{ status: 400 },
49+
);
50+
51+
await expect(assertOkOrThrowProviderError(response, "OAuth token exchange failed")).rejects
52+
.toThrow(
53+
"OAuth token exchange failed (400): AADSTS7000215: Invalid client secret provided. [code=invalid_request]",
54+
);
55+
});
56+
57+
it("keeps HTTP status metadata when error body reads fail", async () => {
58+
const response = {
59+
ok: false,
60+
status: 503,
61+
headers: new Headers(),
62+
body: {
63+
getReader: () => ({
64+
read: async () => {
65+
throw new Error("broken response stream");
66+
},
67+
cancel: async () => undefined,
68+
}),
69+
},
70+
} as unknown as Response;
71+
72+
await expect(assertOkOrThrowProviderError(response, "Provider API error")).rejects
73+
.toMatchObject({
74+
name: "ProviderHttpError",
75+
status: 503,
76+
statusCode: 503,
77+
message: "Provider API error (503)",
78+
} satisfies Partial<ProviderHttpError>);
79+
});
80+
4281
it("attaches structured provider error metadata", async () => {
4382
const response = new Response(
4483
JSON.stringify({

0 commit comments

Comments
 (0)