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

Feature #34766

Better error message when no API format is recognised

Added by Felix Schäfer over 1 year ago. Updated 2 months ago.

Status:

Closed

Start date:

Priority:

Normal

Due date:

Assignee:

Marius BALTEANU

% Done:

Category:

REST API

Target version:

5.0.0

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)

add_test.patch (987 Bytes) Mizuki ISHIKAWA, 2022-01-04 03:42

Related issues

Associated revisions

Revision 21392
Added by Marius BALTEANU 4 months ago

Return 406 status code instead of 500 when API request has an invalid format (#34766).

Patch by Felix Schäfer.

Revision 21393
Added by Marius BALTEANU 4 months ago

Add test for #34766.

Patch by Mizuki ISHIKAWA.

History

#1 Updated by Pavel Rosický over 1 year 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.

#2 Updated by Felix Schäfer over 1 year 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.