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

Exception within transition not raised #34

Open
nburt opened this issue Nov 12, 2015 · 6 comments
Open

Exception within transition not raised #34

nburt opened this issue Nov 12, 2015 · 6 comments

Comments

@nburt
Copy link

nburt commented Nov 12, 2015

When writing tests for some after_transition callbacks I haven't seen exceptions being raised within the callback.

Example:

after_transition, :on => :finish, :do => :finish_it

def finish_it
  foo
end

I'd expect a NameError to be raised but the test continues execution. Is this intended behavior?

@rosskevin
Copy link
Member

Take a look at the test cases like this one that halts a trasition. If you are seeking to disallow the transition on an action error, you need to use before_transition instead of after_transition.

Does this describe your desired behavior?

If you see something misbehaving or you would like it to yield more information (perhaps this case), submitting even just a test-only pull request is helpful as well.

@nburt
Copy link
Author

nburt commented Nov 12, 2015

I'm actually not expecting an exception to be thrown in the actual execution of the after_transition.

While writing the after_transition callback I defined a new method so I expected the test to fail with that exception raised. The test did fail, but due to some expectations at the end of my test. I thought it would have halted the after transition and the test would have failed earlier. Just led to some confusion on my part as I used Pry to figure out what was happening.

Does that make sense?

@rosskevin
Copy link
Member

It does make sense, but I think the nature of a callback is that it will not fail for the execution code, but it certainly would be nice if it would at least dump to stderr if there was a problem. It looks like you are seeking a behavioral change wherein the after_transition would raise. I admit this has bitten me before.

I'm not sure of the reasoning or expectations on the after_transition, but I did find the related test (it was actually a misnamed file, I committed a fix on master).

You might redirect the stderr in the test and assert a message in the content, and submit a PR. Or, we could discuss a behavioral change. @seuros, do you have thoughts on the behavior of after_transition?

@obie
Copy link

obie commented Oct 6, 2017

Bump! I keep running into cases where server hangs if there's an error in a transition, with no indication as to what is happening. Wondering if anyone else has had these kinds of problems. When running standalone rails server it's not that big a deal, but when running in foreman, have to kill -9 the ruby process. Super annoying.

@stuxcrystal
Copy link

stuxcrystal commented May 6, 2019

Bump! You really shouldn't do Pokemon Exception handling.

Nobody is perfect. Web-Applications can be huge. Both are not inductive to a bug-free program. If your code throws an exception because of a bug, you need to know about it in order to fix it and, preferably only to do some damage control or recover the state of the program.

If you silence the exception you signal the program, "everything went fine". Except it didn't and now you have to deal with incosistent state in your program and/or database because your code suppressed the exception that would have prevented commiting the new state to persistent storage.

This cannot be any type of intended behaviour.

Hmm. Does this error still persist?

@rafaelfranca
Copy link
Contributor

Good question. Can you try to reproduce in the test suite?

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

No branches or pull requests

5 participants