Skip to content

Commit 19ba4f9

Browse files
committed
feat: Get rid of manual flushing
1 parent 8f3fab6 commit 19ba4f9

5 files changed

Lines changed: 182 additions & 333 deletions

File tree

packages/mcp-cloudflare/src/server/index.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,6 @@ const wrappedOAuthProvider = {
8585

8686
const response = await oAuthProvider.fetch(request, env, ctx);
8787

88-
// Flush buffered logs before Worker terminates
89-
ctx.waitUntil(Sentry.flush(2000));
90-
9188
// Add CORS headers to public metadata endpoints
9289
if (isPublicMetadataEndpoint(url.pathname)) {
9390
return addCorsHeaders(response);

packages/mcp-cloudflare/src/server/lib/mcp-handler.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import type { ServerContext } from "@sentry/mcp-core/types";
2121
import type { Env } from "../types";
2222
import { verifyConstraintsAccess } from "./constraint-utils";
2323
import type { ExportedHandler } from "@cloudflare/workers-types";
24-
import * as Sentry from "@sentry/cloudflare";
2524

2625
/**
2726
* ExecutionContext with OAuth props injected by the OAuth provider.
@@ -169,10 +168,6 @@ const mcpHandler: ExportedHandler<Env> = {
169168
const server = buildServer({
170169
context: serverContext,
171170
agentMode: isAgentMode,
172-
onToolComplete: () => {
173-
// Flush Sentry events after tool execution
174-
ctx.waitUntil(Sentry.flush(2000));
175-
},
176171
});
177172

178173
// Run MCP handler - context already captured in closures

packages/mcp-core/src/server.ts

Lines changed: 77 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,10 @@ import {
8282
*/
8383
export function buildServer({
8484
context,
85-
onToolComplete,
8685
agentMode = false,
8786
tools: customTools,
8887
}: {
8988
context: ServerContext;
90-
onToolComplete?: () => void;
9189
agentMode?: boolean;
9290
tools?: Record<string, ToolConfig<any>>;
9391
}): McpServer {
@@ -99,7 +97,6 @@ export function buildServer({
9997
configureServer({
10098
server,
10199
context,
102-
onToolComplete,
103100
agentMode,
104101
tools: customTools,
105102
});
@@ -119,13 +116,11 @@ export function buildServer({
119116
function configureServer({
120117
server,
121118
context,
122-
onToolComplete,
123119
agentMode = false,
124120
tools: customTools,
125121
}: {
126122
server: McpServer;
127123
context: ServerContext;
128-
onToolComplete?: () => void;
129124
agentMode?: boolean;
130125
tools?: Record<string, ToolConfig<any>>;
131126
}) {
@@ -254,104 +249,100 @@ function configureServer({
254249
params: any,
255250
extra: RequestHandlerExtra<ServerRequest, ServerNotification>,
256251
) => {
257-
try {
258-
// Get active span (mcp.server span) and attach more attributes to it
259-
const activeSpan = getActiveSpan();
260-
261-
if (activeSpan) {
262-
if (context.constraints.organizationSlug) {
263-
activeSpan.setAttribute(
264-
"sentry-mcp.constraint-organization",
265-
context.constraints.organizationSlug,
266-
);
267-
}
268-
if (context.constraints.projectSlug) {
269-
activeSpan.setAttribute(
270-
"sentry-mcp.constraint-project",
271-
context.constraints.projectSlug,
272-
);
273-
}
274-
}
252+
// Get active span (mcp.server span) and attach more attributes to it
253+
const activeSpan = getActiveSpan();
275254

276-
if (context.userId) {
277-
setUser({
278-
id: context.userId,
279-
});
255+
if (activeSpan) {
256+
if (context.constraints.organizationSlug) {
257+
activeSpan.setAttribute(
258+
"sentry-mcp.constraint-organization",
259+
context.constraints.organizationSlug,
260+
);
280261
}
281-
if (context.clientId) {
282-
setTag("client.id", context.clientId);
262+
if (context.constraints.projectSlug) {
263+
activeSpan.setAttribute(
264+
"sentry-mcp.constraint-project",
265+
context.constraints.projectSlug,
266+
);
283267
}
268+
}
284269

285-
try {
286-
// Apply constraints as parameters, handling aliases (e.g., projectSlug → projectSlugOrId)
287-
const applicableConstraints = getConstraintParametersToInject(
288-
context.constraints,
289-
tool.inputSchema,
290-
);
270+
if (context.userId) {
271+
setUser({
272+
id: context.userId,
273+
});
274+
}
275+
if (context.clientId) {
276+
setTag("client.id", context.clientId);
277+
}
291278

292-
const paramsWithConstraints = {
293-
...params,
294-
...applicableConstraints,
295-
};
279+
try {
280+
// Apply constraints as parameters, handling aliases (e.g., projectSlug → projectSlugOrId)
281+
const applicableConstraints = getConstraintParametersToInject(
282+
context.constraints,
283+
tool.inputSchema,
284+
);
296285

297-
const output = await tool.handler(paramsWithConstraints, context);
286+
const paramsWithConstraints = {
287+
...params,
288+
...applicableConstraints,
289+
};
298290

299-
if (activeSpan) {
300-
activeSpan.setStatus({
301-
code: 1, // ok
302-
});
303-
}
291+
const output = await tool.handler(paramsWithConstraints, context);
304292

305-
// if the tool returns a string, assume it's a message
306-
if (typeof output === "string") {
307-
return {
308-
content: [
309-
{
310-
type: "text" as const,
311-
text: output,
312-
},
313-
],
314-
};
315-
}
316-
// if the tool returns a list, assume it's a content list
317-
if (Array.isArray(output)) {
318-
return {
319-
content: output,
320-
};
321-
}
322-
throw new Error(`Invalid tool output: ${output}`);
323-
} catch (error) {
324-
if (activeSpan) {
325-
activeSpan.setStatus({
326-
code: 2, // error
327-
});
328-
activeSpan.recordException(error);
329-
}
293+
if (activeSpan) {
294+
activeSpan.setStatus({
295+
code: 1, // ok
296+
});
297+
}
330298

331-
// CRITICAL: Tool errors MUST be returned as formatted text responses,
332-
// NOT thrown as exceptions. This ensures consistent error handling
333-
// and prevents the MCP client from receiving raw error objects.
334-
//
335-
// The logAndFormatError function provides user-friendly error messages
336-
// with appropriate formatting for different error types:
337-
// - UserInputError: Clear guidance for fixing input problems
338-
// - ConfigurationError: Clear guidance for fixing configuration issues
339-
// - ApiError: HTTP status context with helpful messaging
340-
// - System errors: Sentry event IDs for debugging
341-
//
342-
// DO NOT change this to throw error - it breaks error handling!
299+
// if the tool returns a string, assume it's a message
300+
if (typeof output === "string") {
343301
return {
344302
content: [
345303
{
346304
type: "text" as const,
347-
text: await formatErrorForUser(error),
305+
text: output,
348306
},
349307
],
350-
isError: true,
351308
};
352309
}
353-
} finally {
354-
onToolComplete?.();
310+
// if the tool returns a list, assume it's a content list
311+
if (Array.isArray(output)) {
312+
return {
313+
content: output,
314+
};
315+
}
316+
throw new Error(`Invalid tool output: ${output}`);
317+
} catch (error) {
318+
if (activeSpan) {
319+
activeSpan.setStatus({
320+
code: 2, // error
321+
});
322+
activeSpan.recordException(error);
323+
}
324+
325+
// CRITICAL: Tool errors MUST be returned as formatted text responses,
326+
// NOT thrown as exceptions. This ensures consistent error handling
327+
// and prevents the MCP client from receiving raw error objects.
328+
//
329+
// The logAndFormatError function provides user-friendly error messages
330+
// with appropriate formatting for different error types:
331+
// - UserInputError: Clear guidance for fixing input problems
332+
// - ConfigurationError: Clear guidance for fixing configuration issues
333+
// - ApiError: HTTP status context with helpful messaging
334+
// - System errors: Sentry event IDs for debugging
335+
//
336+
// DO NOT change this to throw error - it breaks error handling!
337+
return {
338+
content: [
339+
{
340+
type: "text" as const,
341+
text: await formatErrorForUser(error),
342+
},
343+
],
344+
isError: true,
345+
};
355346
}
356347
},
357348
);

0 commit comments

Comments
 (0)