Skip to content

Improve error messages #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 28, 2023
Merged

Improve error messages #40

merged 2 commits into from
Jul 28, 2023

Conversation

ohakutsu
Copy link
Member

@ohakutsu ohakutsu commented Jul 21, 2023

What

エラー発生時のメッセージがわかりにくい(何をすればいいのかわからない。なぜエラーになっているのかわからない)ため、改善する。

How

  • throwされたエラーの出力を改善
  • バリデーションエラーの表示を修正

Why

Refs

@ohakutsu ohakutsu self-assigned this Jul 21, 2023
@ohakutsu ohakutsu force-pushed the improve-error-message branch from 11fa9ca to d88f68e Compare July 25, 2023 03:25
@ohakutsu ohakutsu marked this pull request as ready for review July 25, 2023 03:26
@ohakutsu
Copy link
Member Author

Screenshots

認証が失敗した場合

スクリーンショット 2023-07-26 11 53 32

portが空いていない場合

スクリーンショット 2023-07-26 11 54 25

バリデーションエラー

スクリーンショット 2023-07-26 11 55 41

slideパラメータ

スクリーンショット 2023-07-26 11 56 27 スクリーンショット 2023-07-26 12 01 11

@ohakutsu ohakutsu requested review from a team and getty104 and removed request for a team July 26, 2023 03:06
@ohakutsu ohakutsu force-pushed the improve-error-message branch from d88f68e to 1d2131a Compare July 26, 2023 03:10
Comment on lines 71 to 73
let text = chalk.red.bold(`${msg.name}: `);
text += chalk.red(err);
console.error(text);
Copy link
Member

Choose a reason for hiding this comment

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

IMO&NITS
↓のような書き方の方が直感的に何をやっているかわかりやすいと感じました。

Suggested change
let text = chalk.red.bold(`${msg.name}: `);
text += chalk.red(err);
console.error(text);
const errorName = chalk.red.bold(msg.name);
const errorDescription = chalk.red(err);
console.error(`${errorName}: ${errorDescription}`)

@ohakutsu ohakutsu requested a review from getty104 July 27, 2023 04:37
Copy link
Member

@getty104 getty104 left a comment

Choose a reason for hiding this comment

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

コメントしましたが、どちらでも良いのでLGTMです!

invalidItemMessages.forEach((msg) => {
console.error(msg.name, msg.errors);
msg.errors.forEach((err) => {
const errorName = chalk.red.bold(msg.name + ":");
Copy link
Member

Choose a reason for hiding this comment

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

nits

Suggested change
const errorName = chalk.red.bold(msg.name + ":");
const errorName = chalk.red.bold(`${msg.name}:`);

@ohakutsu ohakutsu force-pushed the improve-error-message branch from a1d1368 to 9468550 Compare July 28, 2023 01:33
@ohakutsu ohakutsu merged commit c8c16fd into main Jul 28, 2023
@ohakutsu ohakutsu deleted the improve-error-message branch July 28, 2023 01:35
@ohakutsu ohakutsu mentioned this pull request Jul 28, 2023
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