Skip to content

Commit 408fc10

Browse files
authored
fix: DH-20166: Log all export errors. (#7116) (#7130)
Cherry-pick of #7116.
1 parent 492c5a3 commit 408fc10

File tree

1 file changed

+35
-18
lines changed

1 file changed

+35
-18
lines changed

server/src/main/java/io/deephaven/server/session/SessionState.java

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public interface Factory {
105105
* @return a sessionless export object
106106
*/
107107
public static <T> ExportObject<T> wrapAsExport(final T export) {
108-
return new ExportObject<>(export);
108+
return new ExportObject<>(export, null);
109109
}
110110

111111
/**
@@ -117,8 +117,7 @@ public static <T> ExportObject<T> wrapAsExport(final T export) {
117117
* @return a sessionless export object
118118
*/
119119
public static <T> ExportObject<T> wrapAsFailedExport(final Exception caughtException) {
120-
ExportObject<T> exportObject = new ExportObject<>(null);
121-
exportObject.caughtException = caughtException;
120+
ExportObject<T> exportObject = new ExportObject<>(null, caughtException);
122121
return exportObject;
123122
}
124123

@@ -603,7 +602,7 @@ private ExportObject(
603602
*
604603
* @param result the object to wrap in an export
605604
*/
606-
private ExportObject(final T result) {
605+
private ExportObject(final T result, final Exception caughtException) {
607606
super(true);
608607
this.errorTransformer = null;
609608
this.session = null;
@@ -614,7 +613,7 @@ private ExportObject(final T result) {
614613
this.logIdentity = Integer.toHexString(System.identityHashCode(this)) + "-sessionless";
615614

616615
if (result == null) {
617-
maybeAssignErrorId();
616+
maybeAssignErrorId(caughtException, null);
618617
state = ExportNotification.State.FAILED;
619618
} else {
620619
state = ExportNotification.State.EXPORTED;
@@ -711,7 +710,7 @@ private synchronized void setWork(
711710

712711
// since this is the first we know of the errorHandler, it could not have been invoked yet
713712
if (errorHandler != null) {
714-
maybeAssignErrorId();
713+
maybeAssignErrorId(caughtException, null);
715714
errorHandler.onError(state, errorId, caughtException, failedDependencyLogIdentity);
716715
}
717716
return;
@@ -860,7 +859,7 @@ private synchronized void setState(final ExportNotification.State state) {
860859
}
861860

862861
if (isExportStateFailure(state) && errorHandler != null) {
863-
maybeAssignErrorId();
862+
maybeAssignErrorId(caughtException, null);
864863
try {
865864
final Exception toReport;
866865
if (caughtException != null && errorTransformer != null) {
@@ -1013,11 +1012,7 @@ private void doExport() {
10131012
if (caughtException != null) {
10141013
synchronized (this) {
10151014
if (!isExportStateTerminal(state)) {
1016-
maybeAssignErrorId();
1017-
if (!(caughtException instanceof StatusRuntimeException)) {
1018-
log.error().append("Internal Error '").append(errorId).append("' ")
1019-
.append(caughtException).endl();
1020-
}
1015+
maybeAssignErrorId(caughtException, null);
10211016
setState(ExportNotification.State.FAILED);
10221017
}
10231018
}
@@ -1032,9 +1027,35 @@ private void doExport() {
10321027
}
10331028
}
10341029

1035-
private void maybeAssignErrorId() {
1030+
private void maybeAssignErrorId(final Exception caughtException, final String errorDetails) {
10361031
if (errorId == null) {
10371032
errorId = UuidCreator.toString(UuidCreator.getRandomBased());
1033+
1034+
if (caughtException == null && errorDetails == null) {
1035+
// We log the assigned error ID, even though we have no details. If we do not assign the error ID,
1036+
// then we may not correctly propagate than an error occurred.
1037+
log.error().append("Internal Error '").append(errorId).append("' for ").append(logIdentity)
1038+
.append(" and no error details are available.").endl();
1039+
return;
1040+
}
1041+
1042+
this.caughtException = caughtException;
1043+
if (!(caughtException instanceof StatusRuntimeException)) {
1044+
if (errorDetails != null) {
1045+
log.error().append("Internal Error '").append(errorId).append("' ").append(errorDetails).endl();
1046+
} else {
1047+
log.error().append("Internal Error '").append(errorId).append("' ").append(caughtException)
1048+
.endl();
1049+
}
1050+
} else {
1051+
if (errorDetails != null) {
1052+
log.info().append("Export failed with Status Runtime Exception '").append(errorId).append("' ")
1053+
.append(errorDetails).endl();
1054+
} else {
1055+
log.info().append("Export failed with Status Runtime Exception '").append(errorId).append("' ")
1056+
.append(caughtException).endl();
1057+
}
1058+
}
10381059
}
10391060
}
10401061

@@ -1063,12 +1084,8 @@ private synchronized void onDependencyFailure(final ExportObject<?> parent) {
10631084
break;
10641085
}
10651086

1066-
maybeAssignErrorId();
1087+
maybeAssignErrorId(caughtException, errorDetails);
10671088
failedDependencyLogIdentity = parent.logIdentity;
1068-
if (!(caughtException instanceof StatusRuntimeException)) {
1069-
log.error().append("Internal Error '").append(errorId).append("' ").append(errorDetails)
1070-
.endl();
1071-
}
10721089
}
10731090

10741091
setState(terminalState);

0 commit comments

Comments
 (0)