grape icon indicating copy to clipboard operation
grape copied to clipboard

`halt` for instantly returning a response, interrupting current handler execution

Open jcagarcia opened this issue 2 years ago • 6 comments

Hey 👋

Do you think a halt method can be interesting for instantly returning a response, interrupting current handler execution? An alternative to error! ?

Something like:

class MyAPI < Grape::API
  get '/wadus' do
    halt 401 unless authenticated?
  end
end

I think it can be useful and it's a standard. Also, it's present in some many other frameworks:

https://github.com/alisnic/nyny#request-scope https://github.com/jeremyevans/roda/blob/master/lib/roda/plugins/halt.rb https://github.com/hanami/controller#throwable-http-statuses

Thanks!

jcagarcia avatar Nov 20 '23 11:11 jcagarcia

You correctly point out that we have error! that behaves this way by raising an exception. I don't think an alias to halt! is needed, but is there a scenario you would want this without an exception? Also I am not sure how that would be implementable (inside an API you can do next).

dblock avatar Nov 20 '23 12:11 dblock

I was thinking more about naming.

When using the error! message, I'm assuming that I'm going to return a error response code. It's really useful to call this method in the middle of a execution because you don't need to deal with returns or if/else conditions.

So I was thinking if you want to return a response that is not an error itself, maybe a 204 if some condition happens and you don't want to deal with if/else conditions and including body and status in each of the if blocks.

It's true that I can use error!({}, 201) and return a not error status code when using the error! (maybe this should not be allowed?), so that was the reason of offering a error! alias for the use cases you don't want to explicitly return an error.

Anyway, if you consider that this is not something you want for grape, it's totally fine :) feel free to close the issue.

jcagarcia avatar Nov 20 '23 13:11 jcagarcia

First, thank you. I (we) are open to suggestions and we absolutely should discuss it. Adding an API is a big deal so expect push back, but if you believe this is the right thing, keep arguing for it, don't give up so easily!

A question, does this work?

status 201
next

Is success! a better name?

Do you have an example where halt! reads significantly better than doing if/else?

dblock avatar Nov 20 '23 23:11 dblock

Hi @dblock ! Thanks for the suggestion :) really appreciated!

I've just tried using the next and it perfectly works.

As I previously said, my issue/concern was more about naming, and as I've seen that other frameworks are using halt for instantly returning a response, that was my suggestion. However, success! or other alternative is a good naming too!

I was trying to build a example where the halt! reads significantly better than doing if/else. However is true that as you need to pass all the status, body and headers parameters to the helper (whatever name we consider), is the same to do this:

if something
  status 201
  header {}
  body {}
  next
end

and to do something like this:

halt!|success!|other!(201, {}, {}) if something

So don't know if it reads significantly better, because at the end you need to manage if conditions as well.

Totally open to add what you think that provides more value to the project, so at this point you have much more context and vision of the project than me.

jcagarcia avatar Nov 21 '23 09:11 jcagarcia

Good points. I think success! could be a good idea semantically, but it would need to throw an exception to abort flow, so it's weird that internally we raise an error to signal success. Something like halt! that accepts a different set of parameters than error! is maybe a good idea after all. Want to try to PR it?

I'd like to hear what others have to say.

dblock avatar Nov 21 '23 14:11 dblock

Sure! Totally open to discuss it and to find the better approach :) I can wait a little bit to provide a PR to see if we hear from others.

On the other hand, do you think that error! should limit the status codes that can be passed?

Thank you so much @dblock !!

jcagarcia avatar Nov 21 '23 14:11 jcagarcia