Skip to content

refactor(apollo-common): 异常处理增加根本原因信息 #5367

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.

Sign up for GitHub

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

Merged
nobodyiam merged 4 commits into from
Apr 1, 2025
Merged

refactor(apollo-common): 异常处理增加根本原因信息 #5367

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dfd539e
refactor(apollo-common): 异常处理增加根本原因信息
youngzil Mar 26, 2025
7bf6e7f
Merge branch 'master' into feature/add-root-cause-info-to-exceptionha…
youngzil Mar 31, 2025
88415b3
refactor(apollo-common): 优化异常处理信息展示
youngzil Mar 31, 2025
49a13d6
Update CHANGES.md
nobodyiam Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Apollo 2.5.0
* [Feature: Provide a new configfiles API to return the raw content of configuration files directly](https://github.com/apolloconfig/apollo/pull/5336)
* [Feature: Enhanced instance configuration auditing and caching](https://github.com/apolloconfig/apollo/pull/5361)
* [Feature: Provide a new open API to return the organization list](https://github.com/apolloconfig/apollo/pull/5365)
* [Refactor: Exception hander adds root cause information](https://github.com/apolloconfig/apollo/pull/5367)
Comment thread
nobodyiam marked this conversation as resolved.
Outdated

------------------
All issues and pull requests are [here](https://github.com/apolloconfig/apollo/milestone/16?closed=1)
13 changes: 12 additions & 1 deletion ...main/java/com/ctrip/framework/apollo/common/controller/GlobalDefaultExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
import org.springframework.core.NestedExceptionUtils;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
Expand All @@ -53,6 +54,7 @@

@ControllerAdvice
public class GlobalDefaultExceptionHandler {

private Gson gson = new Gson();
private static Type mapType = new TypeToken<Map<String, Object>>() {
}.getType();
Expand Down Expand Up @@ -115,7 +117,7 @@ private ResponseEntity<Map<String, Object>> handleError(HttpServletRequest reque

private ResponseEntity<Map<String, Object>> handleError(HttpServletRequest request,
HttpStatus status, Throwable ex, Level logLevel) {
String message = ex.getMessage();
String message = getMessageWithRootCause(ex);
printLog(message, ex, logLevel);

Map<String, Object> errorAttributes = new HashMap<>();
Expand Down Expand Up @@ -169,4 +171,13 @@ private void printLog(String message, Throwable ex, Level logLevel) {
Tracer.logError(ex);
}

private String getMessageWithRootCause(Throwable ex) {
String message = ex.getMessage();
Throwable rootCause = NestedExceptionUtils.getMostSpecificCause(ex);
if (rootCause != ex) {
message += " [Cause: " + rootCause.getMessage() + "]";
}
return message;
}
Comment on lines +174 to +181

@coderabbitai coderabbitai Bot Mar 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

❓ Verification inconclusive

Consider adding unit tests for the new functionality.

This method significantly improves exception handling by providing root cause information. To ensure this functionality works correctly under various scenarios (nested exceptions, null messages, etc.), please add comprehensive unit tests.

🏁 Script executed:

#!/bin/bash
# Check if there are unit tests covering the new exception handling logic

# Try to find test files that might test the GlobalDefaultExceptionHandler
find . -path "*/test/*" -name "*GlobalDefaultExceptionHandler*Test.java" -o -name "*ExceptionHandler*Test.java"

# Look for test changes in the PR
git diff --name-only HEAD~1 HEAD | grep -E "Test\.java$" | xargs cat 2>/dev/null | grep -A 10 -B 10 "getMessageWithRootCause\|rootCause message"

Length of output: 248

Action Required: Add Comprehensive Unit Tests for Exception Handling

The method getMessageWithRootCause in apollo-common/src/main/java/com/ctrip/framework/apollo/common/controller/GlobalDefaultExceptionHandler.java (lines 174–181) now enhances exception handling by appending the root cause message. However, our investigation did not reveal any unit tests covering this functionality. To ensure robustness, please add tests addressing scenarios such as:

  • Nested Exceptions: Verify that when an exception has a distinct root cause, its message is correctly appended.
  • Null Messages: Confirm that the method handles cases where either the primary exception or its root cause has a null message.
  • Identical Exceptions: Ensure that there’s proper handling when the root cause is the same as the original exception.

These tests will help maintain reliable error handling across various failure modes.


}