RequiredParam results in 500 about goliath HOT 10 CLOSED

For the 500 error do you have

use Goliath::Rack::Params
use Goliath::Rack::ValidationError

before the validation middleware?

from goliath.

thanks, adding Goliath::Rack::ValidationError fixes it. I think this ticket should be left open to note that this dependency should be addressed in an automatic way.

from goliath.

I think changing the response to a 422 makes sense. I don't think auto-injecting Goliath::Rack::ValidationError necessarily makes sense as, you could write your own validation middleware that logs to a database or sends an email, or does any number of things. I don't think we can assume that people always want ValidationError.

from goliath.

I think the default should be to inject the dependency. If more flexibility is required a user can add a parameter to the use statement to indicate that they have a custom technique of dealing with errors.

Perhaps exception handling is not the best default either. Why can't the middleware just return a 4xx response be default? That way there is no dependency required. An upstream middleware can still recognize errors and deal with them as desired.

from goliath.

I agree, this has burned me before as well.

Since existing validators are Goliath specific, custom handling of those errors seems like overkill (yagni). ValidationError should either be auto-included by default, or, deleted alltogether and merged into the actual validators.

from goliath.

Is this along the lines of what you're thinking: https://github.com/postrank-labs/goliath/compare/validation

from goliath.

That looks great! And it is still easy enough to override for custom handling or throwing exceptions back to a custom handler. The only problem left is executing the switch to the new api. If i understand things correctly, everything will still just work, but can a warning be given when the old middleware is used now? Or do we just want to add documentation somewhere?

from goliath.

+1, that looks much better. I wish there was a clear rule to distinguish the two.. but I would have made it a mixin. Half a dozen of one, and half a dozen of the other...

I wouldn't be too worried about changing the API at this point. Still a young project, we can afford to make some breaking changes before we get bogged down in legacy support. :-)

from goliath.

My rule of thumb is if the choice is even (and if you can choose), then always use a module. There is really no difference between the 2 anyways- a module behaves exactly like a superclass. But you can only inherit from one class, so you might as well guard that until it provides a benefit. Definitely not worth the effort of changing.

from goliath.

Ok, branch is merged into master. I converted to a module. I wasn't sure which I wanted when I started and the module ended up working better.

Let me know if anything seems wonky.

from goliath.