Feature #34766: Better error message when no API format is recognised - Redmine

Actions

Copy link

Feature #34766

closed

Better error message when no API format is recognised

Added by Felix Schäfer about 3 years ago. Updated about 2 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Marius BĂLTEANU

Category:

REST API

Target version:

5.0.0

Start date:

Due date:

% Done:

Estimated time:

Resolution:

Description

At Planio sometimes see users trying to make API requests using the HTTP Accept: application/json header but forgetting to add .json at the end of the path. This currently leads to a raise with a 500 Internal Server Error.

We propose the following patch to correctly return a 406 Not Acceptable (this is the same HTTP status Rails uses when the passed Accept HTTP header can not be served), log the error instead of raise-ing, and returning a helpful message in the HTTP response.

diff --git a/lib/redmine/views/builders.rb b/lib/redmine/views/builders.rb
index 653c1bd6b..3c977df67 100644
--- a/lib/redmine/views/builders.rb
+++ b/lib/redmine/views/builders.rb
@@ -30,7 +30,9 @@ module Redmine
             when 'xml',  :xml  then Builders::Xml.new(request, response)
             when 'json', :json then Builders::Json.new(request, response)
             else
-              raise "No builder for format #{format}" 
+              Rails.logger.error "No builder for format #{format.inspect}" 
+              response.status = 406
+              return "We couldn't handle your request, sorry. If you were trying to access the API, make sure to append .json or .xml to your request URL.\n" 
             end
           if block_given?
             yield(builder)

Files

add_test.patch (987 Bytes) add_test.patch

Mizuki ISHIKAWA, 2022-01-04 03:42

Related issues

Actions

Copy link

Updated by Pavel Rosický about 3 years ago

well, I had the same proposal 3 years ago https://redmine.org/issues/26709

I don't see a reason why not to provide a JSON response if "Accept: application/json" header is present. I agree, that the application shouldn't raise 500, 406 is definitely more appropriate.

Actions

Copy link

Updated by Felix Schäfer about 3 years ago

Pavel Rosický wrote:

well, I had the same proposal 3 years ago https://redmine.org/issues/26709

Thank you for pointing this out, I was not aware this had already been worked on, I am sorry for overlooking it.

I don't see a reason why not to provide a JSON response if "Accept: application/json" header is present.

Providing a JSON or XML response when the Accept header is set would also be coherent with the way Rails handles requests I think. This would be my preferred solution, but I am unsure whether not accepting the Accept header in Redmine::Builders was a conscious decision or not.

I agree, that the application shouldn't raise 500, 406 is definitely more appropriate.

This would indeed be the smallest possible change that would avoid raising an error in the logs/in the application and improving the HTTP response for users.

Actions

Copy link