fix: define an actionable-error policy and surface those errors as toasts (#131) - #134
Merged
Merged
Conversation
…asts Errors were logged but never shown to the user: the per-level "logERR" socket event the client's onLogErr toast handlers listen for stopped being emitted when the socketIO appender was unified onto "WHEEL_LOG", and the run path's ack(err) is a dropped second ack (projectOperator already resolved it with ack(true)). Whether an error should interrupt the user was also never defined - .error() covered both "you must fix this" and "internal / auto-retried". Policy (documentMD/design/error_handling.md #4): - notifyUser(projectRootDir, ...): the user must act on it (an operation they requested failed, or a setting must be fixed) -> logged at error level AND emitted on "logERR" so the client shows a snackbar. - logger.error() / logError(): worth recording, no user action needed (auto-retried transfer, internal assertion, deprecated API call, a task failure already shown red on its component) -> log only. notifyUser() lives in logSettings.js. It logs via getLogger().error() (so WHEEL_LOG / the log file are unchanged) and emits "logERR" to the project room, or "default" for messages not tied to a project (project list / remotehost screen / import dialog). Errors are reduced to their message for the toast; the full entry stays in the log. Audited every error-level call in server/app and converted the actionable ones (~30 sites): run/save/stop failures in projectController, import/export/upload/rename/commit failures in file/component/project handlers, "host not found" and connection-test failures, invalid project names, editor-op failures, unknown job scheduler, and the stage-out-stuck "re-run to continue" message. Left as log-only: file-browser read failures, gfarm/remote listing errors, internal asserts, deprecated-API calls, per-task errors. No client change. Refs #131 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu
AGENTS.md requires JSDoc on new functions. No behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu
Covers: logERR toast emitted alongside the log, Error reduced to its message (no stack) in the toast, and "default" room fallback for messages not tied to a project. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #131
背景と目的
プロジェクトの実行・再開が dispatch 前または dispatch 中に失敗した場合、サーバー側のログには記録されるものの、クライアント（ブラウザ）側には何も表示されず、利用者が失敗に気づけない状態となっていました。原因は以下の 2 点です。
logERRなど）を送出する処理が、ログ画面刷新時に単一のWHEEL_LOGイベントへ統合された際に失われ、クライアントのonLogErr（トースト表示）ハンドラーが発火しなくなっていた。runProject系の処理ではrunDispatcherを待ち合わせずprojectOperatorが先にack(true)を返すため、catch節のack(err)は Socket.IO の二重 ack となり無視されていた。加えて、「そのエラーで利用者の作業を止めるべきか」という基準が定義されておらず、
logger.error()が「利用者が対処すべきもの」と「内部的・自動リトライ中のもの」の両方に混在して使われていました。変更方針
エラー通知のポリシーを次のとおり定義し、
documentMD/design/error_handling.mdの第 4 章に明記しました。notifyUser(projectRootDir, ...)logERRイベント）logger.error()/logError()実装内容（サーバー側のみ、クライアント変更なし）
server/app/logSettings.jsにnotifyUser()を追加しました。内部でgetLogger(projectRootDir).error(...)を呼び出すため、ログ画面（WHEEL_LOG）およびログファイルへの出力は従来どおりです。これに加えて、対象ルーム（projectRootDir。プロジェクトに紐づかないものは"default"）へlogERRイベントを送出します。トーストに表示する文字列ではErrorをmessageのみに縮約し、スタックトレースは含めません（全文はログ画面・ログファイルで確認できます）。server/app配下のエラーレベルのログ呼び出しをすべて確認し、利用者の対処が必要な箇所（約 30 か所）をnotifyUser()へ置き換えました。主な対象は次のとおりです。projectController：実行 / 保存 / 停止処理の失敗、検証・準備・ワークフロー解析フェーズの失敗remoteFileBrowser：リモートホスト未登録、ダウンロード失敗tryToConnect：接続テストの失敗projectOperations：不正なプロジェクト名、Git リポジトリへのアクセス不可executerManager：未定義のジョブスケジューラー指定dispatcher：stage-out 滞留による実行不能（再実行を促すメッセージ。従来のlogFatalから変更）上記 2 点目の「
ack(err)が無視される」問題は、該当するcatch節がnotifyUser()経由でlogERRを直接送出するようになったため、個別の対応は不要となりました。テスト・Lint
notifyUser()の JSDoc を追加しました（AGENTS.md の規約に準拠）。notifyUser()のユニットテストをserver/test/app/Logging.jsに追加しました（logERRトーストの送出、Errorをmessageのみに縮約すること、プロジェクトに紐づかない場合の"default"ルームへのフォールバック）。eslint）を実行し、本変更による新規の指摘が 0 件であることを確認しました（既存の指摘は本 PR の対象外です）。test/app/Logging.js、test/app/core/restart.js、test/app/core/dispatcher.js、test/app/core/executerManager.js（logErrorのスタブをnotifyUserのスタブへ追随修正）test/app/core/projectOperations.js、test/app/handlers/projectController.js、test/app/handlers/fileManager.js、test/app/handlers/workflowEditor.js関連
onLogErrハンドラー内の正規表現による整形処理は、旧ログ書式を前提としたものであり本変更により不要となりますが、クライアント側の整理は本 PR の対象外とし、別途対応します。🤖 Generated with Claude Code
https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu