Copilot commented on code in PR #59585:
URL: https://github.com/apache/doris/pull/59585#discussion_r2663472528
##########
fe/fe-core/src/main/java/org/apache/doris/common/NereidsException.java:
##########
@@ -23,22 +23,42 @@
public class NereidsException extends RuntimeException {
private final Exception exception;
+ private final boolean suppressStackTrace;
public NereidsException(Exception cause) {
+ this(cause, false);
+ }
+
+ public NereidsException(Exception cause, boolean suppressStackTrace) {
this.exception = cause;
+ this.suppressStackTrace = suppressStackTrace;
}
public NereidsException(String message, Exception cause) {
+ this(message, cause, false);
+ }
+
+ public NereidsException(String message, Exception cause, boolean
suppressStackTrace) {
super(message, cause);
this.exception = cause;
+ this.suppressStackTrace = suppressStackTrace;
}
public Exception getException() {
return exception;
}
+ public boolean isSuppressStackTrace() {
+ return suppressStackTrace;
+ }
+
@Override
public String getMessage() {
return exception.getMessage();
}
+
+ @Override
+ public String toString() {
+ return getMessage();
Review Comment:
The overridden toString() method returns only getMessage(), which could lose
important exception type information when this exception is wrapped. Line 126
creates an IllegalStateException with t.toString() as the message, which will
only include the message text for NereidsException, not the exception class
name. This makes debugging harder. Consider either removing the toString()
override or using t.getMessage() explicitly in places where only the message is
needed.
```suggestion
String message = getMessage();
return getClass().getName() + (message != null ? ": " + message :
"");
```
##########
fe/fe-core/src/main/java/org/apache/doris/common/NereidsException.java:
##########
@@ -23,22 +23,42 @@
public class NereidsException extends RuntimeException {
private final Exception exception;
+ private final boolean suppressStackTrace;
public NereidsException(Exception cause) {
+ this(cause, false);
+ }
+
+ public NereidsException(Exception cause, boolean suppressStackTrace) {
Review Comment:
The constructors that accept a suppressStackTrace parameter don't call the
superclass constructor with the cause parameter. This means the standard
exception chaining mechanism is broken for these constructors. The exception's
cause won't be accessible via getCause(), and standard stack trace printing
won't show the causal chain. Consider calling super(cause) or super() as
appropriate in the new constructor at line 32.
```suggestion
public NereidsException(Exception cause, boolean suppressStackTrace) {
super(cause);
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
