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

Provide a standard exception hierarchy for REST end points [SPR-12531] #17136

Closed
spring-projects-issues opened this issue Dec 10, 2014 · 10 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Dec 10, 2014

Adib Saikali opened SPR-12531 and commented

When building a single page application where the client side is storing the majority of state and server is mostly stateless there is a need for a consistent way to handle errors so that the client side javascript can know what to do.

In a JavaScript heavy app with 10K+ Lines of JS or more having every rest end point do it's own thing for errors can mean a huge mess in the client side JavaScript code base.

So most SpringMVC applications I have worked with in the past few years have built a standard way to handle errors so that.

  • JavaScript client knows exactly what to expect from the server when there is an error
  • Server side developers know exactly how to generate an error that the client side code and developers can consume

Benefits of a consistent error handling strategy

  • Server side APIs are easier to consume
  • Server side APIs are easier to code
  • Server side APIs are easier to design
  • It is easier to log and run analytics on API errors

Three part solution

  • Use HTTP status codes correctly
  • Define a standard data structure for representing errors
  • Make sure that no matter what happens API’s calls always return the standard data structure when an error occurs

Wikipedia lists 83 HTTP status codes so there is a lot of confusion as to what http status codes to use on the part of developers. So the idea to use a subset of HTTP status codes to reduce confusion.

There are 3 possible types of codes

  • Everything worked
  • Blame the API
  • Blame the client

Recommended HTTP Status Codes

  • 200 – if the request is processed successfully
  • 500 – if the problem is the server’s fault
  • 400 – if the request is the client’s fault
  • 401 – if the user needs to log in
  • 403 – if the user is logged in but does not have permission
  • 404 – If the resource does not exist

How do we provide the client with more details about the nature of the problem?

Use a secondary API specific error code

  • Complement the http status code with an optional API specific code
  • The API specific error code should be globally unique so that it is easy to search for the code. Use a UUID for the API specific error code

Five Part Standard Error Response

  • HTTP status code
    • Required and must be one of 200, 500, 400, 401, 403, 404
  • API specific error code
    • Optional must be a UUID so that is globally unique and searchable
  • User message
    • Optional plain text message that the UI can display to the user if it wants
  • Developer message
    • Optional plain text message aimed at developers to explain why the request failed
  • Validation messages
    • Optional array of field validation messages

Here are some example error messages that would be produced

GET: /api/customers/1
response:

{  
   "httpStatus":401,
   "apiCode":"cdbfa4c6-9ae6-46da-aef7-fa167f689afe",
   "userMessage":"Your account was signed out for your security, because of a lack of activity.",
   "developerMessage":"The server side http session has expired, user needs to login in again",
   "validationErrors":null
}

POST: /api/customers, {"name":"Jane","email":"jane@mail.com"}
response:

{  
   "httpStatus":400,
   "apiCode":"062952b8-257f-4fb5-9baf-2d2932375f9d",
   "userMessage":"An Error has occured and the request could not be completed",
   "developerMessage":"Invalid request body see validation errors",
   "validationErrors":[  
      {  
         "fieldName":"name",
         "message":"name must be 10 characters long"
      }
   ]
}

The git repo at https://bitbucket.org/asaikali/springone-2014/ contains a demo implementation of these features from my Talk at SpringOne 2014.

As a server side developer I want to be able to simply throw an exception that produces the standard error message for example.

@RequestMapping(ApiUrls.CUSTOMER)
	public CustomerJson get(@PathVariable("id") Integer id)
	{
		CustomerJson customerJson = this.customers.get(id);
		if(customerJson == null) {
			
			throw new ResourceNotFoundRestApiException()
					  .userMessage("User id %s does not exist",id)
					  .developerMessage("wrong user id the url");
		}
	    return customerJson;
	}

The implementation of this end user experience is located in the git repo at ttps://bitbucket.org/asaikali/springone-2014/ it is a spring boot application just clone it and run the SpaApplication then go to http://localhost:8080 and login using the username/password combo user/password and try out the features.

To best understand the code look at the classes in the following order:

  • com.example.rest.api.RestApiHttpStatus
  • com.example.rest.api.RestApiError
  • com.example.rest.api.RestApiValidationError
  • com.example.rest.api.RestApiException
  • com.example.rest.api.RestControllerAdvice
  • com.example.rest.api.ApiErrorCodes

Then look at the exception classes in the package com.example.rest.api.exceptions

To see what the end user experience is like look at the class com.example.spa.customer.CustomerJson and notice that it has an @Valid annotation on one of the fields then go to com.example.spa.customer.CustomerController and look at the post(@RequestBody @Valid CustomerJson customerJson) method and notice that the standard error message json gets generated if the posted json fails validation. Then look at get(@PathVariable("id") Integer id) to see how a developer will explicitly throw a standard error.

Given the opinionated nature of this request, maybe SpringBoot is a better home for it.


Affects: 4.1 GA

Attachments:

Issue Links:

Referenced from: commits spring-attic/spring-framework-issues@0cb6d39

2 votes, 7 watchers

@spring-projects-issues
Copy link
Collaborator Author

Adib Saikali commented

slide deck from my presentation at Spring One 2014. It covered more than just what is requested in this ticket.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

We'll give this a look for 4.2.

@spring-projects-issues
Copy link
Collaborator Author

Adib Saikali commented

My SpringOne Presentation video should be at http://www.infoq.com/presentations/spring-4-single-page-app

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Adib Saikali, I've had a closer look at this. The idea of a unified, consistent REST API error reporting indeed applies to all applications. That said I struggle to see much more that we can do in the framework.

I've created an example project. It has an @ControllerAdvice that extends ResponseEntityExceptionHandler in order to cover Spring MVC specific exceptions and in a real application it would contain more @ExceptionHandler methods for application specific exceptions. It's a slightly simplified version of your sample code but it makes it easy to see the relevant parts.

I know most REST APIs need the same kinds of fields in the response body but still the exact fields could vary by name or type. Even if we did provide RestApiError and RestApiException the value added is so little it's not worth it IMO.

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Rossen, perhaps I haven't looked closely enough at the available options (and perhaps I haven't understood the details of this issue well enough either), but I think that what I ultimately want with out-of-the-box "RESTful HTTP exception handling" is basically an continuation of what the framework already does out of the box with certain exceptions already. Consider what happens when a HttpRequestMethodNotSupportedException is thrown, e.g.:

$ http DELETE http://localhost:8080
HTTP/1.1 405 Method Not Allowed
Allow: GET, HEAD
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: keep-alive
Content-Type: application/json;charset=UTF-8
Date: Sun, 22 Feb 2015 07:11:14 GMT
[...]

{
    "error": "Method Not Allowed",
    "exception": "org.springframework.web.HttpRequestMethodNotSupportedException",
    "message": "Request method 'DELETE' not supported",
    "path": "/",
    "status": 405,
    "timestamp": 1424589074392
}

What I want (or what I think I want, anyway), is just more exceptions like this that I can throw (or possibly extend and throw), such I never need to hand-craft a custom ResourceNotFoundException again, such that I don't need to create a custom @ExceptionHandler for it, etc. I would just throw ResourceNotFoundException and by default a response very similar to the one above would be output.

You mention above that adding a standard hierarchy of this sort would "add little value" because the likelihood of needing to modify the content of the response body is fairly high. I guess I'd argue that it's still much better to have a hierarchy already there, and one that I can incrementally begin customizing if and when necessary, as opposed to having to (usually badly and incompletely) re-create that hierarchy in each application that I build.

Hopefully I'm not missing some big point here. Thanks for looking into this in any case.

@spring-projects-issues
Copy link
Collaborator Author

Adib Saikali commented

Rossen any thoughts on adding something like this to SpringBoot? Should this ticket be moved to the Spring Boot project?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Feb 23, 2015

Rossen Stoyanchev commented

Chris, I think what you're asking is a little different. This ticket is about providing a generic REST exception that contains instructions for how to set the status and the body of the response. The intent is to codify common best practices around REST error handling for what information should be available in the response body to make a REST API that's developer friendly.

What you're asking for is closer to #16177. You can follow the discussion there but the basic premise is that the web layer (e.g. @ExceptionHandler methods) should be translating business exceptions to REST API responses. A controller method however might still need to raise something like a ResourceNotFoundException if the underlying API returns null which needs to be translated into a 404. We do have ResponseStatusExceptionResolver which supports exceptions annotated with @ResponseStatus so you can add the exceptions you need, annotate them with @ResponseStatus, and there should be no need to write exception handling code.

We could provide a few exceptions around the common status codes but it's hard to decide where to draw the line and it's so easy for the application to do. I suppose we could provide a generic ResponseStatusException that accepts status code in the constructor as a middle ground. Still in many cases I would expect the REST API to want to provide additional detail, so one way or another many applications would have to come up with some kind of Rest API error exception as requested in this ticket that reflects their needs.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

It reminds me that we provided in Resthub Spring Stack this kind of behavior, see JpaHandlerExceptionHandler and ResthubExceptionHandler.

Since Resthub is a kind of pre Spring Boot project, and as proposed by Adib Saikali, I think this issue should be closed in favor of a Spring Boot one that could be built thanks some @ControllerAdvice + @ConditionalOnClass beans.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Adib, indeed Spring Boot would be a better place for out-of-the-box REST API errors that include a body with an API code, user, and developer messages. The key question again is going to be the question about flexibility around it. For example it's almost a given that whatever API codes Boot might assign to internal Spring MVC exceptions would have to be changed by applications. Or if there is a builder for REST API exceptions, what if you need add, remove, or change even one field? If it ends up being not flexible enough to even make basic changes, or if you have to complete replace it, then it's not so appealing even if it works out-of-the-box. That said it's probably worth re-evaluating this request in the context of Boot. Perhaps there are ways to make this natural that I can't see right now.

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Rossen, I see you just closed this issue. Perhaps it's worth noting here that since I've started using Spring Data REST, I no longer care about a standard exception hierarchy, because the errors that SD REST creates out of the box are generally exactly what I need / expect. Perhaps others will find the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants