Skip to content
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

Disable display of detailed exceptions #296

Conversation

pedrosmmoreira
Copy link
Contributor

@allow-rescue permits rescuing exceptions by setting action_dispatch.show_exceptions to true in the env hash. However, this will render the detailed error page and not the public error page you'd encounter in production.

Given that the feature enables testing of error pages, and seems to be intended to mirror production behaviour as per the comment on https://github.com/cucumber/cucumber-rails/blob/master/lib/generators/cucumber/install/templates/support/_rails_each_run.rb.erb#L6-L14, this PR sets action_dispatch.show_detailed_exceptions to false, thus enabling the public error page to be rendered.

@Kosmas
Copy link
Member

Kosmas commented Jun 4, 2015

@pedrosmmoreira thank you for the PR, but the tests are not passing. Can you please try and get them to pass first?

@pedrosmmoreira
Copy link
Contributor Author

@Kosmas Absolutely, I'm working on it :)

@pedrosmmoreira
Copy link
Contributor Author

@Kosmas This PR depends on action_dispatch.show_detailed_exceptions being set, which was only introduced in Rails 3.2.1, as far as I can tell. Hence, the appraisal for previous versions is failing and breaking the build. How would you like me to proceed?

@Kosmas
Copy link
Member

Kosmas commented Jun 9, 2015

@pedrosmmoreira thanks for having a look.

I think it would make sense to have a check for the rails version, and then depending on > or < 3.2.1 set the show_detailed_exceptions. What do you think?

@Kosmas
Copy link
Member

Kosmas commented Dec 15, 2015

@pedrosmmoreira would you like to try again now that the tests are fixed?

@pedrosmmoreira
Copy link
Contributor Author

@Kosmas Sure, I'll rebase and check if we can clear this.

@Kosmas
Copy link
Member

Kosmas commented Jan 21, 2016

@pedrosmmoreira can you please rebase so it can run again.
It should be working now.

@pedrosmmoreira
Copy link
Contributor Author

@Kosmas Done. Sorry for the delay on this.

@Kosmas
Copy link
Member

Kosmas commented Jan 21, 2016

@pedrosmmoreira no worries.. we had some problems with the tests anyway, but let's see if they work now!

@mathieujobin
Copy link
Contributor

@pedrosmmoreira can you rebase with master to make sure all tests passes?
thanks

pedrosmmoreira and others added 3 commits August 5, 2016 23:11
`@allow-rescue` permits rescuing exceptions by setting
`action_dispatch.show_exceptions` to true in the env hash. However,
this will render the detailed error page and not the public error page
you'd encounter in production.

Given that the feature enables testing of error pages, and seems to be
intended to mirror production behaviour as per the comment on
https://github.com/cucumber/cucumber-rails/blob/master/lib/generators/cucumber/install/templates/support/_rails_each_run.rb.erb#L6-L14,
this PR sets `action_dispatch.show_detailed_exceptions` to false, thus
enabling the public error page to be rendered.
There are slight changes in the error message displayed in the public
error pages across Rails versions. Checking for "We're sorry, but
something went wrong." seems to be the safest option.
This PR depends on action_dispatch.show_detailed_exceptions being set,
which was only introduced in Rails 3.2.1.

Here we introduce a conditional check based on the required Rails
version in order to ensure backwards compatibility.
@pedrosmmoreira
Copy link
Contributor Author

@mathieujobin All done - I'm currently traveling but I'll look to see if we can this ready for merge as soon as I am back. Apologies for leaving it neglected so long!

@xtrasimplicity
Copy link
Member

I've rebased these changes onto master, and have created a new PR (#387). I'll close this one off.

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.

None yet

4 participants