500 error with accept-header that has no respond_to block #14362

Closed
jvanbaarsen opened this Issue Mar 12, 2014 · 25 comments

Comments

Projects
None yet
8 participants
Contributor

jvanbaarsen commented Mar 12, 2014

Hi,

I'm using rails 4.0.3, i currently have the following problem:

When I create a HTTP call to my web-app with the following accept-header: Accept: images/jpeg, I get a 500 error because rails cant find any template that matches that accept header.

I expect this to be a 404 error.

Contributor

htanata commented Mar 12, 2014

I'm having this problem on Rails 3.2.17 app too. It can be triggered using:

curl -kv -H "Accept: images/jpeg" http://localhost:3000

This happens when the action doesn't have respond_to block. Adding the code as shown below solves the problem. Rails will return "406 Not Acceptable" instead of an exception.

def index
  respond_to do |format|
    format.html
  end
end

But adding this to all actions looks like a lot of code. I think it will be nice to have a default respond_to setting that can be overridden in each actions if needed.

Contributor

htanata commented Mar 12, 2014

I created a test case for this: https://gist.github.com/htanata/9515581

Contributor

jvanbaarsen commented Mar 12, 2014

@htanata Thanks! ❤️

Contributor

larrylv commented Jun 13, 2014

Hi, @htanata and @jvanbaarsen

I don't think Rails should have a default respond_to setting.

When you specify respond_to, you specify the response mime for different request header, but if request contains a unknown header, actionpack will raise a 406 error.

But when you don't specify respond_to, actionpack will try to render a template for this request, when there are no related templates to return for the header, actionview will raise a 500 ActionView::MissingTemplate error.

So I think the solution for you guys should be using rescue_from in your ApplicationController and customize the response.

What do you think, @rafaelfranca

Owner

matthewd commented Jun 13, 2014

I could certainly see an argument that MissingTemplate should ultimately yield a 406 (in production, at least) as a default behaviour.

On the other hand, 406 at least implies that the server's refusing to do what you asked, and if we're hitting MissingTemplate, we've already run the action, so in the general (i.e., non-GET/HEAD) case, we can't really guarantee that. So, there's equally an argument to be made that if we've "done stuff", and now we're realising rather-too-late that we don't know how to respond, an Internal Server Error has in fact occurred.

Contributor

htanata commented Jun 13, 2014

I don't think this is about whether MissingTemplate should return "406 Not Acceptable". I think the current behavior of returning "500 Internal Server Error" for MissingTemplate and returning "406 Not Acceptable" when the client is requesting for unsupported format is correct.

Using rescue_from would hide a real error when I really don't have the template. For example, if I specify that my action responds to HTML, but I don't have the HTML template, then I would like to see 500 error.

The problem is that I need to put respond_to on all my actions to get this behavior, which is a lot of code, or else I'm going to get a lot of exceptions from random bots.

Since using respond_to seems to be optional (as shown in the Getting Started guide), I think having a default respond_to format would be useful for reducing the amount of code needed to avoid 500 errors from invalid requests.

@rails-bot rails-bot added the stale label Nov 19, 2014

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

Contributor

htanata commented Nov 21, 2014

I've just reproduced this on master using https://gist.github.com/htanata/9515581.

Owner

rafaelfranca commented Nov 26, 2014

❤️

Contributor

jvanbaarsen commented Dec 23, 2014

@htanata @rafaelfranca I was looking at this issue trying to create a fix for it. But i'm confused about what is the proposed solution at this point. Can you please explain a little what you guys where thinking for a solution?

Contributor

htanata commented Dec 26, 2014

@jvanbaarsen My idea is to have a controller setting that can be overridden by its subclasses like this, similar to how layout works:

class ApplicationController < ActionController::Base
  only_respond_to :html
end

class User < ApplicationController
  only_respond_to :html, :json
end

If an action has a respond_to block, then only_respond_to will be ignored for that action.

Maybe it's also good that the subclass can turn if off too, for example, by giving false as the argument:

class User < ApplicationController
  only_respond_to false
end

What do you think?

Owner

rafaelfranca commented Dec 26, 2014

This is exactly what the responders feature does and it was extracted from
Rails. I don't think we will add the same feature with other name.

On Fri, Dec 26, 2014, 21:45 Hendy Tanata notifications@github.com wrote:

@jvanbaarsen https://github.com/jvanbaarsen My idea is to have a
controller setting that can be overridden by its subclasses like this,
similar to how layout
http://guides.rubyonrails.org/layouts_and_rendering.html#specifying-layouts-for-controllers
works:

class ApplicationController < ActionController::Base
only_respond_to :htmlend
class User < ApplicationController
only_respond_to :html, :jsonend

If an action has a respond_to block, then only_respond_to will be ignored
for that action.

Maybe it's also good that the subclass can turn if off too, for example,
by giving false as the argument:

class User < ApplicationController
only_respond_to falseend

What do you think?


Reply to this email directly or view it on GitHub
#14362 (comment).

Contributor

htanata commented Dec 27, 2014

It doesn't seem to be the same to me. responders gem has logic like setting flash message and others while my idea is that only_respond_to is equivalent to just adding respond_to at the end of the action:

only_respond_to :html, :json

# Is equivalent to adding this at the end of all actions in the controller:
respond_to do |format|
  format.html
  format.json
end

But if it's possible to get that same effect from responders gem, I don't mind using that, although I think it's better if it's part of Rails. Once your site is live, you're bound to get these errors from bots, unless you put respond_to block in all your actions. Another benefit of having this config is that it allows Rails to reject bad requests early instead of running the code on the action first, only to find that the template doesn't exist.

Contributor

htanata commented Dec 27, 2014

Ah, ignore my previous the last sentence. I realized that's not possible if each individual action can override that settings from within. 😬

Owner

rafaelfranca commented Dec 27, 2014

What you propose is equivalent to respond_to class method that is provided
by responders feature. Pay attention that responders (the feature) was
different from responders (the gem). After Rails 4.2 the gem provides the
feature but it is available on Rails since 3.2

On Fri, Dec 26, 2014, 22:30 Hendy Tanata notifications@github.com wrote:

Ah, ignore my previous the last sentence. I realized that's not possible
if each individual action can override that settings from within. [image:
😬]


Reply to this email directly or view it on GitHub
#14362 (comment).

Contributor

htanata commented Dec 27, 2014

Yeah, I know about respond_to on older Rails and the fact that it's extracted out.

respond_to on older Rails solves this issue somewhat, but you still need to write respond_with on all actions, so it's not exactly the same.

Maybe it's possible to solve this by changing the responders gem instead. I'll look into it.

Contributor

jvanbaarsen commented Jan 5, 2015

@rafaelfranca Maybe that does mean that this issue may not be a real issue? Shall I close it?

Contributor

shivanibhanwal commented Jan 6, 2015

Hi @jvanbaarsen @rafaelfranca
I am upgrading my project from 4.1.8 to 4.2, ruby 2.2
My app is an Rails api app so I do not have any views in it. We are using Active Model Serialiser and all the responses of my controllers are "render json"

e.g.    render json: @data, each_serializer: serializer

Every time I am getting error

Status Code:500 Internal Server Error

When I debug the code this what I see

[34, 43] actionpack-4.2.0/lib/action_controller/metal/instrumentation.rb

When I check "render(*args)" in args I see error something like this:
PG::ProtocolViolation: ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1

When I check further
[18, 26] activerecord-4.2.0/lib/active_record/railties/controller_runtime.rb
def cleanup_view_runtime and debug further deeper I see this message

#<ActionView::MissingTemplate: Missing template api/v1/offers/index, api/v1/api/index, application/index with {:locale=>[:en], :formats=>[:json], :variants=>[], :handlers=>[:erb, :builder, :raw, :ruby]}. Searched in:

I am unable to understand why for the JSON render it is looking for a template

Contributor

jvanbaarsen commented Feb 8, 2015

@shivanibhanwal Im not really sure about this, but isn't it always looking for a template even if you say render json:...?

Owner

kaspth commented Feb 8, 2015

@shivanibhanwal the issue you're seeing isn't the same as this one. From the information you've given here I think it's not likely it's a bug in Rails. Try the Ruby on Rails Talk mailing list or stack overflow, they'll be better able to help you :)

@jvanbaarsen if you have an explicit render call, Rails won't implicitly look for and render a template for the request format.

Contributor

jvanbaarsen commented Feb 9, 2015

@kaspth Thanks! Do you agree that I can close this issues as it seems not be an issue after all?

Owner

kaspth commented Feb 9, 2015

Sure, I'll go ahead and close it for you 😄

@kaspth kaspth closed this Feb 9, 2015

Contributor

jvanbaarsen commented Feb 9, 2015

@kaspth haha ok thanks :)

Contributor

shivanibhanwal commented Feb 9, 2015

Hi @jvanbaarsen @kaspth I have figured it out long time back but forgot to update here. That issue was not due to rails it was due to Postgres ext change..
Thanks for your time 😄

Owner

kaspth commented Feb 9, 2015

Great to hear you figured it out :)

Kasper

Den 09/02/2015 kl. 18.08 skrev Shivani notifications@github.com:

Hi @jvanbaarsen @kaspth I have figured it out long time back but forgot to update here. That issue was not due to rails it was due to Postgres ext change..
Thanks for your time


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment