Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 17 additions & 44 deletions platform/lambda/index.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { handle } from 'hono/aws-lambda';
import { Context, Hono } from 'hono';
import { StreamableHTTPTransport } from '@hono/mcp';
import { Logger } from '@aws-lambda-powertools/logger';
import { BlankEnv, BlankInput } from 'hono/types';
import { toFetchResponse, toReqRes } from 'fetch-to-node';
import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js';
import { createMcpServer } from './mcp-server';
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';

const logger = new Logger();

Expand Down Expand Up @@ -56,54 +56,27 @@ const handleError = (
);
};

const closeResources = async (server: McpServer, transport: StreamableHTTPTransport) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

削除された重要な機能: closeResources 関数は、リソースの安全なクリーンアップを担保する重要な機能でした。この関数が提供していた以下の機能が失われています:

  1. Promise.allSettled による両方のリソースの確実なクリーンアップ
  2. 個別のクローズエラーのログ出力
  3. 一方のクローズが失敗しても他方を実行する堅牢性

この機能を復活させることを強く推奨します。

// 両方のクローズを確実に実行(片方が失敗してももう片方を実行)
const closeResults = await Promise.allSettled([
transport.close(),
server.close(),
]);

// クローズエラーをログ出力
closeResults.forEach((result, index) => {
if (result.status === 'rejected') {
const resourceName = index === 0 ? 'transport' : 'server';
const error = result.reason;
const errorDetails = error instanceof Error
? { message: error.message, stack: error.stack }
: error;
logger.error(`Error closing ${resourceName}:`, { error: errorDetails });
}
});
};

// ルートを設定

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

トランスポートインスタンス化のタイミング: トランスポートを try ブロック内で作成することで、接続エラー時にもトランスポートのクリーンアップが必要になります。元のコードのように、トランスポートを事前に作成し、接続エラー時は適切にクリーンアップする方が安全です。

app.post('/mcp', async (c) => {
const { req, res } = toReqRes(c.req.raw);
const server = createMcpServer();
const transport = new StreamableHTTPTransport({
sessionIdGenerator: undefined, // セッションIDを生成しない(ステートレスモード)
enableJsonResponse: true,
});
try {
const transport = new StreamableHTTPServerTransport({
sessionIdGenerator: undefined, // セッションIDを生成しない(ステートレスモード)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ログレベルの不整合: console.log を使用していますが、他の箇所では logger インスタンスを使用しています。一貫性を保つため、ここでも logger.trace または logger.debug を使用することを推奨します。

enableJsonResponse: true,
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛑 リソースリーク: res.on('close') イベントハンドラーによるリソース管理に重大な問題があります。Lambda環境では、レスポンスの close イベントが適切に発火しない可能性があり、その結果 transport.close()server.close() が呼ばれずにリソースリークが発生する恐れがあります1

元のコードの try-finally パターンの方が確実にリソースクリーンアップを保証できます。現在の実装では、Lambda関数の実行が完了してもMCPサーバーとトランスポートが適切にクローズされない可能性があります。

Footnotes

  1. Lambda環境では、レスポンスストリームの動作が通常のNode.jsサーバーと異なる場合があります

await server.connect(transport);
try {
logger.trace('MCP リクエストを受信');
return await transport.handleRequest(c);
} catch (error) {
return handleError(c, error, 'MCP リクエスト処理中のエラー:');
} finally {
await closeResources(server, transport);
}
logger.trace('MCP リクエストを受信');
await transport.handleRequest(req, res, await c.req.json());
Comment on lines 58 to +70

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

エラーハンドリングの改善が必要: 現在の実装では、transport.handleRequest() でエラーが発生した場合のリソースクリーンアップが不十分です。以下の改善を提案します:

  1. try-catch-finally パターンを使用してリソースの確実なクリーンアップを保証
  2. transport.close()server.close() の呼び出し時のエラーハンドリング
  3. 元のコードにあった Promise.allSettled を使用した安全なクリーンアップロジックの復活
Suggested change
const closeResources = async (server: McpServer, transport: StreamableHTTPTransport) => {
// 両方のクローズを確実に実行(片方が失敗してももう片方を実行)
const closeResults = await Promise.allSettled([
transport.close(),
server.close(),
]);
// クローズエラーをログ出力
closeResults.forEach((result, index) => {
if (result.status === 'rejected') {
const resourceName = index === 0 ? 'transport' : 'server';
const error = result.reason;
const errorDetails = error instanceof Error
? { message: error.message, stack: error.stack }
: error;
logger.error(`Error closing ${resourceName}:`, { error: errorDetails });
}
});
};
// ルートを設定
app.post('/mcp', async (c) => {
const { req, res } = toReqRes(c.req.raw);
const server = createMcpServer();
const transport = new StreamableHTTPTransport({
sessionIdGenerator: undefined, // セッションIDを生成しない(ステートレスモード)
enableJsonResponse: true,
});
try {
const transport = new StreamableHTTPServerTransport({
sessionIdGenerator: undefined, // セッションIDを生成しない(ステートレスモード)
enableJsonResponse: true,
});
await server.connect(transport);
try {
logger.trace('MCP リクエストを受信');
return await transport.handleRequest(c);
} catch (error) {
return handleError(c, error, 'MCP リクエスト処理中のエラー:');
} finally {
await closeResources(server, transport);
}
logger.trace('MCP リクエストを受信');
await transport.handleRequest(req, res, await c.req.json());
const { req, res } = toReqRes(c.req.raw);
const server = createMcpServer();
const transport = new StreamableHTTPServerTransport({
sessionIdGenerator: undefined, // セッションIDを生成しない(ステートレスモード)
enableJsonResponse: true,
});
try {
await server.connect(transport);
logger.trace('MCP リクエストを受信');
await transport.handleRequest(req, res, await c.req.json());
return toFetchResponse(res);
} catch (error) {
return handleError(c, error, 'MCP 接続中のエラー:');
} finally {
// 両方のクローズを確実に実行(片方が失敗してももう片方を実行)
const closeResults = await Promise.allSettled([
transport.close(),
server.close(),
]);
// クローズエラーをログ出力
closeResults.forEach((result, index) => {
if (result.status === 'rejected') {
const resourceName = index === 0 ? 'transport' : 'server';
const error = result.reason;
const errorDetails = error instanceof Error
? { message: error.message, stack: error.stack }
: error;
logger.error(`Error closing ${resourceName}:`, { error: errorDetails });
}
});
}


res.on('close', () => {
console.log('Request closed');
transport.close();
server.close();
});

return toFetchResponse(res);
} catch (error) {
// サーバー接続に失敗した場合、transportのみクローズ(serverは未接続のため)
// この時点でserver(サーバー)は未接続と考えられる。未接続のサーバーに対してクローズ処理を実行すると予期しないエラーが発生する可能性があるため
try {
await transport.close();
} catch (closeError) {
const errorDetails = closeError instanceof Error
? { message: closeError.message, stack: closeError.stack }
: closeError;
logger.error('Transport close failed after connection error:', { closeError: errorDetails });
}
return handleError(c, error, 'MCP 接続中のエラー:');
}
});
Expand Down
1 change: 1 addition & 0 deletions platform/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"@modelcontextprotocol/sdk": "^1.19.1",
"aws-cdk-lib": "^2.219.0",
"constructs": "^10.4.2",
"fetch-to-node": "^2.1.0",
"hono": "^4.9.10",
"zod": "^3.25.76"
}
Expand Down
47 changes: 9 additions & 38 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.