Update:
Adde more examples
Recently I had an experience about a confusing code and inconsistent error responses in a rest-service, written in Java and SpringBoot.
That led me to some experiments and to recap what I learned from programming in GoLang and reading Robert C. Martin's book "Clean Code" which has a whole section dedicated on error handling :D
Bad Examples
I regularly stumble accross code that logs the stacktrace at every step in the code, sometimes even without a proper log-message. This results in multiple times the same stacktrace.
The controller:
@RestController
@AllArgsConstructor
@Slf4j
public class HelloController {
private final UserService userService;
@GetMapping("/hello")
public Mono<ResponseEntity<User>> hello() {
return userService.getUser()
.map(ResponseEntity::ok)
.defaultIfEmpty(ResponseEntity.notFound().build())
.onErrorResume(e -> {
log.error("Error:", e);
return Mono.error(e);
});
}
}
And in the Service:
@Service
@AllArgsConstructor
@Slf4j
public class UserService {
private final UserRepository userRepository;
/**
* Reads some data from another service or repo and returns it.
*/
public Mono<User> getUser() {
return userRepository.getUser()
.map("Hello %s"::formatted)
.onErrorResume(e -> {
log.error("Error:", e);
return Mono.error(e);
});
}
}
- Makes very large logs with thousands of log-lines for one error, just because of multiple times that the stacktrace was printed.
- Confuses the error-investigation, was it one exception? Multiple ones? Which is the correct one?
- Missing error-messages? Without a meaningful error message it's very hard to find the reason why the error occured.
My Rule
So what I came up with is nothing but new, this is just a recap and something I consider a good practice when I do my reviews on someone elses code.
It boils down to one rule and three steps when you handle an error (or exception).
The rule:
"Handle the error where it occurs."
The three steps
- Log a meaningful error message (and the exception message) once, but only once!
- Assess whether you can recover from here, then handle it here and continue without passing on exceptions or error codes.
- Fail Fast: If you cannot recover, throw an "unchecked", specific, new exception and with that terminate the request, do not propagate the caught exception. Handle the unchecked exception in a global error handler.
Here in more detail and reasoning.
1. Log a meaningful message
It is crucial, to think extensively what to log so someone investigating an error can directly see what happened, where and why. In most cases this should be done without printing the whole stacktrace, if you need to print the stacktrace, than print it preferably in debug logs, so if neccessary the debug-mode can be activated to see a more detailed picture.
Avoid printing multiple logs for the same error! This obfuscates the code, makes it hard to detect where exactly the error occurred and what the original reason was.
2. Assess whether you can recover
Catch the exception where it occurs, then check if you can recover from that exception, e.g. you can continue without that information or perform a retry for a connection. This is crucial to keep the code clean and have your logic there where you need it.
If you can recover, then do it and don't pass on the exceptions or error codes, error-codes are anyway a bad idea as mentioned in Robert C. Martin's book.
3. Fail Fast
If you cannot recover, then don't just propagate the caught exception.
Instead, wrap it in a specific, new, "unchecked" exception that will terminate the current request.
This helps to see very exactly where the error appeared, it helps to keep your code clean by not having to handle the exception in every method in the stacktrace. Instead use a global error handler to catch those specific exceptions and return a meaningful response to the client.
My custom exception
public class UserNotFoundException extends RuntimeException {
public UserNotFoundException(String message, Throwable cause) {
super(message, cause);
}
}
Wrapping the original exception
public Mono<User> getHello() {
return userRepository.getUser()
.onErrorMap(e -> new UserNotFoundException("Error while fetching user data from repository", e));
}
And finally taking care of the exceptions in the global error-handler
@RestControllerAdvice
public class GlobalErrorHandler {
/**
* Handling unexpected, non-specific exceptions
*/
@ExceptionHandler(Throwable.class)
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
public ErrorResponse handleUncaughtExceptions(Throwable t) {
log.error("Uncaught exception occurred: {}", t.getMessage());
log.debug("Stacktrace", t);
return new ErrorResponse(t.getMessage(), "ERR_500");
}
/**
* Handle UserNotFoundException
*/
@ExceptionHandler(UserNotFoundException.class)
@ResponseStatus(HttpStatus.NOT_FOUND)
public ErrorResponse handleUserNotFoundException(UserNotFoundException e) {
log.warn("User could not be found in the database, request cannot be processed - {}", e.getMessage());
return new ErrorResponse("Unable to find user", "ERR_404");
}
}
Using this, will keep all upstream methods clean, because they don't have to implement try-catch and error-handling again and again, but can focus on the main task and logic.
This also reduces the amount of log-lines printed for each error and makes finding the root-cause much easier.
Summary
Using this three-step method helps you to keep your code clean, to have meaningful logs that improve investigation if something fails and avoids overboarding log entries that obfuscate the main issue.
If you want further reading on Java Exception Handling, you may enjoy the Error Handling Bad Practices article.
Top comments (0)