Skip to content

Conversation

@kigawa01
Copy link
Member

エラー通知に汎用的なエラーメッセージを追加

概要

task-client-protocol-details.md のエラー通知セクションに汎用エラーのJSON例を追加しました。

背景・目的

汎用エラーが特定のタスクに紐付かないため、システム全体のエラーを表現する新しいメッセージタイプを追加する必要がありました。これにより、エラーハンドリングの柔軟性が向上します。

変更内容

  • generic_errorタイプのメッセージ例を追加
  • エラーメッセージをシステムレベルのものに変更

確認方法

spec/task-client-protocol-details.md を確認し、新しい汎用エラー通知の例が追加されていることを確認してください。

その他

なし

- task-client-protocol-details.md を新規作成
- task-client-protocol.md から通信プロトコル詳細セクションを移動
- 元のファイルに参照リンクを追加
@kigawa01 kigawa01 force-pushed the add-generic-error-notification branch from 0d22fb0 to 15a3efe Compare December 25, 2025 15:39
@claude
Copy link

claude bot commented Dec 25, 2025

PR レビュー: エラー通知に汎用的なエラーメッセージを追加

全体評価

汎用エラー通知の追加という目的は理解できますが、いくつかの重要な問題と改善点があります。

🐛 発見された問題

1. .gitignore の重複エントリ (Critical)

問題箇所: .gitignore:2

 .idea/
+.idea/

問題点:

  • .idea/ が重複して追加されています
  • この変更はPRの本来の目的と無関係です

修正方法:
この変更を削除してください。


2. メッセージタイプの一貫性の欠如 (High)

問題箇所: spec/task-client-protocol-details.md:118

問題点:

  • 既存のエラー通知は type: "task_error" を使用していますが、新しい汎用エラーは type: "generic_error" を使用しています
  • プロトコル仕様全体で、すべてのメッセージタイプは task_ プレフィックスを持っています (例: task_execute, task_status_update, task_completed)
  • generic_error はこの命名規則から外れています

提案:

  1. type: "task_error" を使用し、taskId フィールドを省略するか null にする
  2. または、新しいタイプが本当に必要な場合は type: "task_generic_error" に変更する

実装例 (オプション1 - 推奨):

{
  "type": "task_error",
  "data": {
    "status": "FAILED",
    "errorCode": "GENERIC_ERROR",
    "errorMessage": "システムで予期しないエラーが発生しました",
    "exitCode": -1,
    "retryable": false,
    "failedAt": "2024-01-01T10:08:00Z"
  },
  "timestamp": "2024-01-01T10:08:00Z"
}

3. プロトコル仕様との整合性不足 (Medium)

問題箇所: spec/task-client-protocol.md

問題点:

  • task-client-protocol.md:106-116 にエラーコードのリストがありますが、GENERIC_ERROR が含まれていません
  • 新しいエラータイプを追加する場合、上位レベルの仕様ドキュメントにも反映する必要があります

必要な対応:
spec/task-client-protocol.md の エラーコード セクションに GENERIC_ERROR を追加してください:

### エラーコード
- **CONNECTION_LOST**: 接続切断
- **AUTH_FAILED**: 認証失敗
- **INVALID_MESSAGE**: 不正メッセージ
- **TASK_NOT_FOUND**: タスク不存在
- **TASK_ALREADY_EXISTS**: タスク重複
- **TASK_IN_PROGRESS**: 実行中タスクの操作不可
- **PERMISSION_DENIED**: 権限不足
- **VALIDATION_FAILED**: 入力検証失敗
- **EXECUTION_FAILED**: 実行失敗
- **RESOURCE_EXHAUSTED**: リソース不足
- **OPERATION_NOT_SUPPORTED**: サポート外操作
+ **GENERIC_ERROR**: 予期しないシステムエラー

また、汎用エラーメッセージの表 (123-134行目) にも追加してください。


4. ドキュメント構造の問題 (Low)

問題箇所: spec/task-client-protocol-details.md:115

問題点:

  • ##### 汎用的なエラー通知例 というサブセクションタイトルは、既存の構造と一貫性がありません
  • 他のメッセージタイプは #### N. メッセージタイプ名 という形式で番号付けされています

提案:
以下のいずれかの構造に変更してください:

オプションA: エラー通知セクション内のサブ例として配置

#### 5. エラー通知

##### タスク固有のエラー例
[既存のコード]

##### システムレベルのエラー例
[新しいコード]

オプションB: 独立したメッセージタイプとして配置

#### 5. タスクエラー通知
[既存のコード]

#### 6. システムエラー通知
[新しいコード]

#### 7. タスクキャンセル要求
[以前の「6. タスクキャンセル要求」]

📋 ベストプラクティスの提案

1. エラーメッセージの詳細化

現在のエラーメッセージ「システムで予期しないエラーが発生しました」は、デバッグに十分な情報を提供していません。

提案:

  • errorDetails フィールドを追加してスタックトレースやコンテキスト情報を含める
  • または、より具体的なエラーコードを使用する (例: INTERNAL_SERVER_ERROR, NETWORK_ERROR)

2. ユースケースの文書化

このgeneric_errorメッセージがどのような状況で使用されるべきかを文書化することを推奨します。

追加すべき説明の例:

##### 汎用的なエラー通知例

特定のタスクに紐付かないシステムレベルのエラーが発生した場合に使用されます。
例: WebSocket接続の初期化失敗、認証後のシステムエラー、プロバイダー登録時のエラーなど。

✅ 良い点

  1. 明確な目的: タスクに紐付かないエラーを表現する必要性は理解できます
  2. 構造の一貫性: 新しいメッセージは既存のエラーメッセージと同じ構造を持っています
  3. 適切なフィールド: retryable: falseexitCode: -1 は汎用エラーとして適切です

🎯 推奨される修正手順

  1. .gitignore の重複エントリを削除
  2. ✅ メッセージタイプを task_error に変更するか、命名規則に従って task_generic_error に変更
  3. spec/task-client-protocol.md にエラーコードと汎用メッセージを追加
  4. ✅ ドキュメント構造を改善 (オプションAまたはBを選択)
  5. (オプション) エラーの使用シーンを文書化
  6. (オプション) errorDetails フィールドの追加を検討

📊 コード品質スコア

  • 機能性: 6/10 - 目的は達成されているが、一貫性に問題あり
  • ドキュメント品質: 5/10 - 不完全な仕様更新
  • 保守性: 4/10 - 命名規則の不一致と構造の問題
  • 全体: 5/10 - 修正が必要

修正後の再レビューを推奨します。

@kigawa01 kigawa01 merged commit 38cda43 into main Dec 25, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants