Display exception messages using simple_format #40567
Merged
Conversation
This file contains 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
hahmed
force-pushed
the
ha/formatted-error-messages
branch
from
06bbaa9 to
b3b7ba7
Compare
|
When you are done, can you please post screeshots of before and after? |
hahmed
force-pushed
the
ha/formatted-error-messages
branch
from
b3b7ba7 to
618d0ae
Compare
exception message.
hahmed
force-pushed
the
ha/formatted-error-messages
branch
from
618d0ae to
93f0ceb
Compare
|
The "negative" I have found with this is, when we are displaying errors like we have with syntax errors, they are also displayed using simple_format on multiple lines... 😅-- is that an acceptable tradeoff? Before:
After:
|
hahmed
commented
Nov 9, 2020
| @@ -299,7 +300,7 @@ def foo | |||
| app("development") | |||
| get "/rails/mailers/notifier" | |||
| assert_predicate last_response, :not_found? | |||
| assert_match "Mailer preview 'notifier' not found", last_response.body | |||
| assert_match "Mailer preview 'notifier' not found", h(last_response.body) | |||
There was a problem hiding this comment.
Unfortunately without this change, this test was failing on rails 2.7. If I updated to "Mailer preview 'notifier' not found" this test failed on rails 2.6.5
Example of test failure without this change on 2.7
Failure:
ApplicationTests::MailerPreviewsTest#test_mailer_preview_not_found [test/application/mailer_previews_test.rb:303]:
Expected /Mailer\ preview\ &\#39;notifier&\#39;\ not\ found/ to match # encoding: ASCII-8BIT
Html from mailer (snipped)
<h1>Unknown action</h1>\n</header>\n<div id=\"container\">\n <div class=\"exception-message\">\n <div class=\"message\">Mailer preview 'notifier' not found</div>\n </div>\n\n</div>\n\n\n</body>\n</html>\n".
rafaelfranca
added a commit
that referenced
this pull request
Nov 9, 2020
Display exception messages using simple_format
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Currently error messages are displayed all in one line in the error pages, in the rails console/logs they appear nicely on multiple lines, making it easier to grok. With this PR I want to propose that we display error messages in the view on multiple lines making it easier for developers to read the error messages.
These changes were pulled out from #39723
Before:
After: