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

Allow exception handling to be disabled #6

Merged
merged 2 commits into from
Mar 19, 2014

Conversation

sidonath
Copy link
Contributor

The default exception handling is very inconvenient while developing and testing a controller as it may hide difficult to spot mistakes behind a very generic 500 error. A solution proposed in #5 would help in development, but not when writing tests.

An afterthought: handle_exceptions is very similar to the handled_exceptions attribute and handle_exception method of Lotus::Action::Throwable so it may cause unintentional problems with typos. I'll gladly fix the PR if any better name is suggested.

The default exception handling is very inconvenient in development mode
as it may hide difficult to spot errors behind a very generic 500 error.
@jodosha
Copy link
Member

jodosha commented Mar 18, 2014

@sidonath Thanks for the PR, I'm gonna merge it, because it's coherent with the actual design.

But before, I have two concerns that I'd like to discuss:

  1. In the future, I'd like to remove these class level configurations. They are handy for developers, but they are singletons within the context of a Ruby process. As now, if you want to have two or more Lotus apps, in the same process, you have to fight those configurations.
  2. This proposed configuration will create a divergence between production and dev/testing behavior. Where possible, I'd like to keep Lotus behaving the same, across the environments. Do you thing it's feasible during testing to look at env['rack.errors'] instead of expecting an exception to be raised?

@jodosha
Copy link
Member

jodosha commented Mar 18, 2014

A further clarification about point 2.

Sometimes during testing, you can't tell if the software is behaving correctly from its output, you may need to obtain this information elsewhere. Kent Beck, in his TDD book, suggests a Log String pattern. In this case env['rack.error'] will act as a logger for Controller tests.

@sidonath
Copy link
Contributor Author

@jodosha

  1. Would you like me to try modifying the PR in order to explore possibility of adding these settings on an object basis? Maybe create a class method handle_exceptions on Controller and Action? Although I'm not sure how to make this an option easily switchable for all controllers/actions in a single place without monkey patching.
  2. I find it very helpful for objects to raise an exception when an error is encountered (e.g. misspelling a variable/method name). Otherwise, almost every every unit test of an action would need to ensure that a status 2xx was returned. Of course, this could be done in a global after block for all action tests, but now we would need very specialized test suites (like minitest-rails) which, to me, seems even worse. I believe it's valid for an app to behave (slightly) different in dev and production environments.

@jodosha
Copy link
Member

jodosha commented Mar 18, 2014

@sidonath Thanks for the feedback. My concern about point 1 isn't about this PR, but about Lotus in general.

I was thinking to something like this: https://gist.github.com/jodosha/9621213

jodosha added a commit that referenced this pull request Mar 19, 2014
Allow exception handling to be disabled
@jodosha jodosha merged commit fc10811 into hanami:master Mar 19, 2014
@jodosha
Copy link
Member

jodosha commented Mar 19, 2014

Merging, thanks!

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

2 participants