enapps-enorman edited a comment on pull request #6:
URL: 
https://github.com/apache/sling-org-apache-sling-servlets-post/pull/6#issuecomment-738899033


   > @kwin @npeltier @enapps-enorman - I think it would be good to have another 
pass at reviewing this PR. I personally think it is an incremental improvement 
and we should include it ; further optimisations can come down the road.
   
   I'm not convinced this change accomplishes much.  It is just replacing one 
guess about what the status code should be with another guess about what the 
status code should be.  The upstream code that throws the exceptions really 
needs to throw more specific exceptions to be able to truly know what the real 
cause was to determine what the status code should be.  For example, 403 for 
AccessDenied, 400 (or 422?) for validation or constraint violations, 409 for 
some concurrency conflicts, 500 for other unexpected errors.
   
   In any case it doesn't look to me that a 405 response code would be right 
for a generic error as it has specific meaning that doesn't seem to apply to 
all the possible use cases here.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to