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

Are you missing that template or did you omit it on purpose? #24037

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
87 changes: 34 additions & 53 deletions actionpack/lib/action_controller/metal/implicit_render.rb
Original file line number Diff line number Diff line change
@@ -1,33 +1,31 @@
require 'active_support/core_ext/string/strip'

module ActionController
# Handles implicit rendering for a controller action when it did not
# explicitly indicate an appropiate response via methods such as +render+,
# +respond_to+, +redirect+ or +head+.
# Handles implicit rendering for a controller action that does not
# explicitly respond with +render+, +respond_to+, +redirect+, or +head+.
#
# For API controllers, the implicit render always renders "204 No Content"
# and does not account for any templates.
# For API controllers, the implicit response is always 204 No Content.
#
# For other controllers, the following conditions are checked:
# For all other controllers, we use these heuristics to decide whether to
# render a template, raise an error for a missing template, or respond with
# 204 No Content:
#
# First, if a template exists for the controller action, it is rendered.
# This template lookup takes into account the action name, locales, format,
# variant, template handlers, etc. (see +render+ for details).
# First, if we DO find a template, it's rendered. Template lookup accounts
# for the action name, locales, format, variant, template handlers, and more
# (see +render+ for details).
#
# Second, if other templates exist for the controller action but is not in
# the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt>
# is raised. The list of available templates is assumed to be a complete
# enumeration of all the possible formats (or variants, etc.); that is,
# having only HTML and JSON templates indicate that the controller action is
# not meant to handle XML requests.
# Second, if we DON'T find a template but the controller action does have
# templates for other formats, variants, etc., then we trust that you meant
# to provide a template for this response, too, and we raise
# <tt>ActionController::UnknownFormat</tt> with an explanation.
#
# Third, if the current request is an "interactive" browser request (the user
# navigated here by entering the URL in the address bar, submiting a form,
# clicking on a link, etc. as opposed to an XHR or non-browser API request),
# <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error
# message.
# Third, if we DON'T find a template AND the request is a page load in a web
# browser (technically, a non-XHR GET request for an HTML response) where
# you reasonably expect to have rendered a template, then we raise
# <tt>ActionView::UnknownFormat</tt> with an explanation.
#
# Finally, it falls back to the same "204 No Content" behavior as API controllers.
# Finally, if we DON'T find a template AND the request isn't a browser page
# load, then we implicitly respond with 204 No Content.
module ImplicitRender

# :stopdoc:
Expand All @@ -37,39 +35,23 @@ def default_render(*args)
if template_exists?(action_name.to_s, _prefixes, variants: request.variant)
render(*args)
elsif any_templates?(action_name.to_s, _prefixes)
message = "#{self.class.name}\##{action_name} does not know how to respond " \
"to this request. There are other templates available for this controller " \
"action but none of them were suitable for this request.\n\n" \
"This usually happens when the client requested an unsupported format " \
"(e.g. requesting HTML content from a JSON endpoint or vice versa), but " \
"it might also be failing due to other constraints, such as locales or " \
"variants.\n"

if request.formats.any?
message << "\nRequested format(s): #{request.formats.join(", ")}"
end

if request.variant.any?
message << "\nRequested variant(s): #{request.variant.join(", ")}"
end
message = "#{self.class.name}\##{action_name} is missing a template " \
"for this request format and variant.\n" \
"\nrequest.formats: #{request.formats.map(&:to_s).inspect}" \
"\nrequest.variant: #{request.variant.inspect}"

raise ActionController::UnknownFormat, message
elsif interactive_browser_request?
message = "You did not define any templates for #{self.class.name}\##{action_name}. " \
"This is not necessarily a problem (e.g. you might be building an API endpoint " \
"that does not require any templates), and the controller would usually respond " \
"with `head :no_content` for your convenience.\n\n" \
"However, you appear to have navigated here from an interactive browser request – " \
"such as by navigating to this URL directly, clicking on a link or submitting a form. " \
"Rendering a `head :no_content` in this case could have resulted in unexpected UI " \
"behavior in the browser.\n\n" \
"If you expected the `head :no_content` response, you do not need to take any " \
"actions – requests coming from an XHR (AJAX) request or other non-browser clients " \
"will receive the \"204 No Content\" response as expected.\n\n" \
"If you did not expect this behavior, you can resolve this error by adding a " \
"template for this controller action (usually `#{action_name}.html.erb`) or " \
"otherwise indicate the appropriate response in the action using `render`, " \
"`redirect_to`, `head`, etc.\n"
message = "#{self.class.name}\##{action_name} is missing a template " \
"for this request format and variant.\n\n" \
"request.formats: #{request.formats.map(&:to_s).inspect}\n" \
"request.variant: #{request.variant.inspect}\n\n" \
"NOTE! For XHR/Ajax or API requests, this action would normally " \
"respond with 204 No Content: an empty white screen. Since you're " \
"loading it in a web browser, we assume that you expected to " \
"actually render a template, not… nothing, so we're showing an " \
"error to be extra-clear. If you expect 204 No Content, carry on. " \
"That's what you'll get from an XHR or API request. Give it a shot."

raise ActionController::UnknownFormat, message
else
Expand All @@ -85,9 +67,8 @@ def method_for_action(action_name)
end

private

def interactive_browser_request?
request.format == Mime[:html] && !request.xhr?
request.get? && request.format == Mime[:html] && !request.xhr?
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trimming is fair, but I don't think we want to ditch the "this might not be a problem" entirely. The "Network Inspector -> Open in New Tab" workflow will be confusing if it sounds like the actually-successful XHR response failed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a swing at that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

end
end