Skip to content

Rambulanceを使って404/500/422ページを整備 #1702

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

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

nacchan99
Copy link
Contributor

@nacchan99 nacchan99 commented Jun 10, 2025

Resolves #1407

🛠 対応内容

これまでカスタムページが表示されていなかった404, 422, 500, エラー、(および400エラー)のページが、それぞれ適切に表示されるように設定しました。

既存のRambulance Gemを有効活用し、エラーページのビューを app/views/errors/ に配置。
また、Rambulanceが提供するプレビュー用ルートを利用することで、開発中の表示確認も容易に行える構成です。


✨ 実装内容

  • 既存のRambulance Gemを有効化し、エラー処理を設定

    • config/initializers/rambulance.rb で、各種エラーのマッピングを設定。
    • 422エラーのビューファイル名をRambulanceの仕様に合わせ、表示問題を修正。
  • エラーページのビューとヘルパーを作成

    • app/views/errors/ 配下に、共通部分 (show.html.erb) と、各ステータスコード用のビューを作成。
    • エラーメッセージを一元管理するためのErrorsHelperを作成。
  • リクエストスペックによる自動テストを追加

    • 上記変更に合わせ、404, 500, 422, 400エラー発生時の挙動を検証するテストを追加。
    • テスト内でErrorsHelperを呼び出すことで、メンテナンス性を向上。

🖥️ 確認方法

開発環境でサーバーを起動後、Rambulanceが提供する以下のプレビュー用ルートにアクセスすることで、各エラーページの表示を確認できます。

  • 404 (Not Found) ページ
    http://localhost:3000/rambulance/not_found

  • 422 (Unprocessable Entity) ページ
    http://localhost:3000/rambulance/unprocessable_content

  • 500 (Internal Server Error) ページ
    http://localhost:3000/rambulance/internal_server_error

  • 400 (Bad Request) ページ
    http://localhost:3000/rambulance/bad_request


~### 📝 残る課題・相談事項~~
テストカバレッジに関して、ErrorsHelperelse句のテストが追加できていない点が課題として残っています。

  • できていること:
    • 開発環境のプレビューでは、/previews/errors/400else句のメッセージが表示されることを確認済み。
      400 (Internal Server Error) ページ http://localhost:3000/previews/errors/400
スクリーンショット 2025-06-19 14 50 13
  • 困っていること:

    • RSpecのテストで400エラーを発生させると、期待する400ではなく500ステータスが返ってきてしまい、テストが失敗します。
  • 試したこと:

    • spring stopによるキャッシュクリア
    • rambulance.rbの設定内容の再確認
      このテスト環境特有の問題について、何か解決のヒントがあればご教示いただきたいです🙌

【解決済み】

上記のテスト環境の問題は、以下の改善を行うことで解決し、全てのテストが成功することを確認しました!

  • 根本原因の解決(ファイル名の修正)
    最大の要因は、Rambulanceが422エラーに対して期待するビューファイル名 (unprocessable_content.html.erb) と、実際のファイル名が異なっていたことでした。
    これを修正したことで、422エラーが正しく処理されるようになりました。

  • テスト手法の改善(レビュー反映)
    また、rakudaさんからのアドバイスに基づき、else句のテストで発生させる例外を、 ActionController::BadRequest (400) に変更しました。
    これにより、テストが安定して動作するようになりました。

これらの修正により、else句を含む全てのエラーページのテストを実装することができました。
@rakuda-san-desu アドバイスありがとうございました!✨


🖼️ 表示確認(スクリーンショット)

404
スクリーンショット 2025-06-10 15 52 30

422
スクリーンショット 2025-06-11 9 34 38

500
スクリーンショット 2025-06-23 10 25 26

400
スクリーンショット 2025-06-23 10 28 06

✅ 現時点でのto doリスト(6/23)

  • 422エラーのビューファイル名を修正する
    Rambulanceのプレビュー機能で422エラーが500として表示される問題を解決するため、ファイル名を unprocessable_entity.html.erb から unprocessable_content.html.erb に変更する。
  • 自作プレビュー機能を削除する
    app/controllers/previews/errors_controller.rb を削除する。
    config/routes.rb から namespace :previews のルーティングを削除する。
  • Rambulance標準プレビューを有効化する
    config/routes.rb で、開発環境でのみ /rambulance がマウントされていることを確認する。
  • テスト用ルーティングを修正する
    spec/requests/errors_spec.rb で、else句のテスト用に ActionController::BadRequest (400) を発生させる /trigger_400 ルートを定義する。
  • テストの検証方法を改善する
    include ErrorsHelper でヘルパーを読み込む。
    expect(response.body).to include("...") のような直接的な文字列比較をやめ、expect(response.body).to include(error_title(404)) のようにヘルパーメソッドを呼び出す形に変更する。
  • テストケースを網羅する
    404, 500, 422 に加え、400エラー(else句)のテストケースを追加し、全てのテストが成功することを確認する。
  • PR説明文を更新する
    これまでの経緯(問題の発見と解決策)を反映した、最終的な説明文に書き換える。
  • スクリーンショットを更新する
    /rambulance で表示した、404, 500, 422, 400 の最終的なスクリーンショットに差し替える。

@yasulab
Copy link
Member

yasulab commented Jun 10, 2025

@nacchan99 PR ありがとうございます!可能そうならテストも追加してもらえると助かります!(>人< )✨(View の文言は頻繁に更新されるので、文言を頻繁に更新してもその都度テストを修正する必要がないテストだと嬉しいです! 🙇 )

@yasulab
Copy link
Member

yasulab commented Jun 10, 2025

あと 500 の部分にも以下の文言を追加していただけると助かります...!! (≧∇≦)b✨ (Partial にしても良いかも?)

image

@nacchan99
Copy link
Contributor Author

@yasulab
コメントありがとうござます!
今日の作業時間内では終わらなかったので、また明日再開します🙌

@nacchan99
Copy link
Contributor Author

@rakuda-san-desu
お疲れ様です!
対応が完了したので、一度ご確認をお願いします🙌

まだRSpec周りは手探りで進めている部分が多く、特にテストの書き方には自信がないので、書き方や構成に違和感があればご指摘いただけると嬉しいです…!🙇‍♀️

お手隙の際によろしくお願いします🙌

@yasulab yasulab self-requested a review June 11, 2025 08:15
</div>
<script async="" await="" src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
</section>
<%= render 'errors/footer_links_and_timeline' %>
Copy link
Member

@yasulab yasulab Jun 11, 2025

Choose a reason for hiding this comment

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

Error 専用の Partial にしては個々に分割しすぎな印象なので (個々に分けるメリットより個々に分けたことよる煩雑さが上回って僕がメンテするとき辛いので)、 共通化できる箇所のみを1つの Partial にまとめる ようにしていただけると助かります!(>人< )✨

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ご指摘ありがとうございます!

errors/headerfeedback_notice はインライン化し、 footer_links_and_timeline のみ partial として残しました。

対応コミット:

  • errors/header のインライン化 ➡︎ 9dac37e
  • errors/feedback_notice のインライン化 ➡︎ c0d871a

ご確認よろしくお願いします!

Copy link
Contributor

@rakuda-san-desu rakuda-san-desu left a comment

Choose a reason for hiding this comment

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

実装ありがとうございます!

いくつかコメントしましたが、動作は良さそうです🙆‍♀️

@@ -0,0 +1,15 @@
class ErrorsController < ApplicationController
layout 'application' # エラー画面にも通常のアプリと同じレイアウトを適用
Copy link
Contributor

@rakuda-san-desu rakuda-san-desu Jun 11, 2025

Choose a reason for hiding this comment

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

このコメントはカットして良さそうです👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ご指摘ありがとうございます!
こちらのコミットで対応済みです🙌 4b52ff7

layout 'application' # エラー画面にも通常のアプリと同じレイアウトを適用

def not_found
render status: 404 # このアクションでは app/views/errors/not_found.html.erb が使用されます
Copy link
Contributor

@rakuda-san-desu rakuda-san-desu Jun 11, 2025

Choose a reason for hiding this comment

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

他所も同様ですが、
該当する行の上にコメントを書くと、他の部分と統一されて良さそうです🛠️

    # このアクションでは app/views/errors/not_found.html.erb が使用されます
    render status: 404 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

統一しました🛠
対応コミットはこちらです→ 7e4737c

with_exceptions_app do
get '/does_not_exist'
end
expect(response.status).to eq(404)
Copy link
Contributor

Choose a reason for hiding this comment

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

「子どものためのプログラミング道場」など、必ず表示されるテキストが存在するかも確認しておくと、さらに安心できそうです👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

テストを追加しました🙌
092c3dd
よろしくお願いします!

nacchan99 added 14 commits June 12, 2025 07:50
- application.rbからを削除
- production.rbとtest.rbの直後にを追加
- 開発環境では従来のバックトレース表示()を維持し、ローカルデバッグ性を確保
- 本番/CI環境では例外を自前ルーティング(,,)へ委譲することで一貫したカスタムエラー画面を提供
- 開発&テスト環境のみで 500/422 エラーを意図的に発生させる ,  ルートを追加
- Rambulance エンジンを開発/テスト環境でのみマウントして、ローカルでエラーページを確認しやすく設定
開発/テスト環境のみにRambulanceをマウントし、“rambulance_engine”ルート名の競合を回避
・開発環境での即時プレビューと、本番環境での動的レンダーを両立
・ErrorsController経由の共通ビュー管理からのアプローチ変更
・Gem『rambulance』の標準ディレクトリを活用し、テンプレート修正をシンプルに
・ErrorsHelperにerror_title/error_descメソッドを追加
・local_assigns[:status_code]をもとに@title/@descをセット
・provideでレイアウト用のタイトル・ディスクリプションを連携
・%w(404 422 500) の match ブロックを削除
・get "/trigger_500" および get "/trigger_422" のトリガールートを削除
・Rambulance::Engine のマウントのみ残すようルーティングを整理
 にビューを配置する構成を試みたが、意図通りに動作しなかった。

 の設定で と指定されているため、 ディレクトリにビューファイルを配置する、よりシンプルで確実な構成に変更。
Rambulanceのレンダリングコンテキストにおいて、
 を使った変数の受け渡しが
意図通りに動作しない問題があったため、
よりシンプルで確実なローカル変数を直接使う方式に変更。

- インスタンス変数(@変数)の使用をやめ、ローカル変数に統一。
- これにより、ビューとヘルパーが正しく連携し、
  500エラーが発生する問題が解消される。
422エラー(InvalidAuthenticityToken)が正しく処理されて

いなかった問題と、サーバー起動時にクラッシュする問題を修正。

- にを追加し、

  422ページが表示されるようにマッピング。

- サーバー起動時のエラーを回避するため、

  全ての例外クラスを定数から文字列に変更。
when 404 then "ページが削除された可能性があります 🤔💭"
when 422 then "入力内容に誤りがあるか、リクエストが正しく送信されなかった可能性があります。"
when 500 then "申し訳ありません。サーバーで問題が発生しています。"
else "しばらく経ってから再度お試しください。"
Copy link
Member

@yasulab yasulab Jun 18, 2025

Choose a reason for hiding this comment

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

@nacchan99 しばらく経っても直らないかもしれないので、ココは別のメッセージの方が良いかもです!👀💡✨

Copy link
Contributor Author

Choose a reason for hiding this comment

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

かしこまりました!
アドバイスありがとうございます🙌✨

Copy link
Member

@yasulab yasulab Jun 18, 2025

Choose a reason for hiding this comment

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

ありがとうございます!書き忘れてしまいましたが、意図としては以下の通りになります!

しばらく経っても直らないかもしれないので、ココは別のメッセージの方が良いかもです!👀💡✨

  • 意図: 通常ではない操作をして発生したエラーなら、通常のフローから外れている可能性もあるので、しばらく経っても直らない。 その場合はエラーの報告をお願いした方が良い 🙏 ✨

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます🙌
その方向で修正します!

開発中に、設定ファイルを変更したり、意図的に例外を
発生させたりすることなく、エラーページのデザインを
簡単に確認できるようにするためのプレビュー機能を追加します。

-  を作成
- 開発環境でのみ有効な  のルートを追加
元から存在したとが、
他のエラーページと異なる独自の構造を持っていたため、
共通のパーシャルを呼び出す形に修正。
エラーの種類ごとにアクションやルートを追加する必要があった
静的なプレビュー機能を、URLのステータスコードを元に
動的にページを出し分ける、よりDRYな構成にリファクタリング。

-  に  アクションを実装
- ルートを  に一本化
- これにより、今後のメンテナンス性が大幅に向上します
Rambulanceで生成されるエラーページ(404, 422, 500)が正しく表示されることを確認するためのリクエストスペックを追加します。

各エラーを意図的に発生させるためのテスト用ルーティングを定義し、それぞれのリクエストで期待されるステータスコードが返却されることを検証します。
@nacchan99 nacchan99 force-pushed the issue-1407-show-error-page branch from 4a1051d to cc5dad9 Compare June 19, 2025 03:36
@nacchan99
Copy link
Contributor Author

@rakuda-san-desu
PRの説明文を、実装内容や確認方法が分かりやすいように更新しました!

また、一番下の「残る課題・相談事項」に、テストで困っていることについて追記しました🙌
お時間がある際に、ご確認いただけると嬉しいです🌼
お忙しいところ恐れ入りますが、よろしくお願いします!

@rakuda-san-desu
Copy link
Contributor

rakuda-san-desu commented Jun 20, 2025

細かい調整ありがとうございます!
確認方法もわかりやすくて助かりました👍

テスト部分も含めて、確認しながらご提案できたらと思う部分があるので、月曜朝などに少しzoomでお話しできたらと思います🙏✨

@rakuda-san-desu
Copy link
Contributor

rakuda-san-desu commented Jun 23, 2025

共有したことメモ

  • 概要欄について
    • 500エラーのスクショを実際に合わせて修正
    • 400エラーのスクショも入れると🙆‍♀️
  • コードについて
    • errors_spec.rb の最下部など、不要な改行をカット
    • 式展開がない場合はシングルクォートが良さそう
    • テストの用のルーティングを以下にして 400 のテストを追加できそう
      - get '/trigger_403', to: ->(env) { raise ActionController::Forbidden }
      + get '/trigger_400', to: ->(env) { raise ActionController::BadRequest }
    • テストでErrorsHelper を読み込んで、expect(response.body).to include(error_title(404)) など、呼び出して評価しても良さそう
  • パスの定義について
    • /previews/errors/404は直感的にプレビューであることやエラーの種類がわかって良い
    • /rambulance/not_found Gemで良い意されたルートなので変更が少なく、rambulance であることがわかって良い ←おすすめ

Rambulanceのプレビュー機能での検証をきっかけに、422エラー発生時に500ページが表示される問題が判明しました。

原因は、Rambulanceが期待するテンプレート名 () と実際のファイル名 () が異なっていたためです。
ファイル名をRambulanceの仕様に合わせ、この問題を修正します。
@yasulab yasulab self-requested a review June 23, 2025 04:46
自作していたエラーページのプレビュー機能 ( と関連ルーティング) を削除しました。

先ほどのビューファイル名の修正により、Rambulanceが  の末尾で定義しているプレビュー用ルート (,  など) で正しく表示できるようになったため、このリファクタリングを実施します。

よりシンプルでメンテナンスしやすい構成になります。
エラーページが正しく表示されることを検証する、
リクエストスペックを追加・改善しました。

- ErrorsHelperを読み込み、具体的な文言ではなく
  ヘルパーの実行結果を検証することで、将来の文言変更に
  強いテストに変更。
- これまでテストできていなかった400エラー(else句)の
  テストケースも追加完了。
レビューでの指摘を反映し、テストコード内で使用する
文字列のクォートを修正しました。
Copy link
Contributor

@rakuda-san-desu rakuda-san-desu left a comment

Choose a reason for hiding this comment

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

丁寧なPRありがとうございます💖

Rambulance 提供のルートを利用する形に変更されたので、不要になった部分にだけコメントしています。それ以外は問題なさそうでしたので、Approve しています!
調整後、マージしていただければと思います🙏

Comment on lines +25 to +27
# テスト環境でも例外を自前ルーティングへ
config.exceptions_app = self.routes

Copy link
Contributor

Choose a reason for hiding this comment

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

Rambulance提供のルートを利用する場合は、この定義も不要そうです👍

Comment on lines +15 to +17
# 本番環境では例外を自前ルーティングへ
config.exceptions_app = self.routes

Copy link
Contributor

Choose a reason for hiding this comment

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

Rambulance提供のルートを利用する場合は、この定義も不要そうです👍

Comment on lines +5 to +9
# Rambulance を開発/テスト環境でのみマウント
if Rails.env.development? || Rails.env.test?
mount Rambulance::Engine => "/"
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Rambulance提供のルートを利用する場合は、この定義も不要そうです👍

Comment on lines +113 to +118

# Rambulance がキャッチする /404, /422, /500
match "/404", to: Rambulance::Engine, via: :all, defaults: { status_code: 404 }
match "/422", to: Rambulance::Engine, via: :all, defaults: { status_code: 422 }
match "/500", to: Rambulance::Engine, via: :all, defaults: { status_code: 500 }

Copy link
Contributor

Choose a reason for hiding this comment

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

Rambulance提供のルートを利用するので、この定義も不要そうです👍

@rakuda-san-desu
Copy link
Contributor

これまでカスタムページが表示されていなかった404, 422, 500, エラー、(および400エラー)のページが、それぞれ適切に表示されるように設定しました。

本番環境でもカスタムページが表示されているはず(404を実際に確認)なので、上記の表現だと少し齟齬がありそうに感じました👀

「カスタムエラーのテンプレートを整理し、それぞれ適切に表示されるように設定しました。」などのほうが、実態に近いかもしれません🙆‍♀️

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.

ローカルでエラーページを表示させたい
3 participants