imbajin commented on code in PR #683:
URL:
https://github.com/apache/incubator-hugegraph-toolchain/pull/683#discussion_r2464757928
##########
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/HugeGraphLoader.java:
##########
@@ -109,42 +206,63 @@ public boolean load() {
// Print load summary
Printer.printSummary(this.context);
} catch (Throwable t) {
+ this.context.occurredError();
+
+ if (t instanceof ServerException) {
+ ServerException e = (ServerException) t;
+ String logMessage =
+ "Log ServerException: \n" + e.exception() + "\n";
+ if (e.trace() != null) {
+ logMessage += StringUtils.join((List<String>) e.trace(),
+ "\n");
+ }
+ LOG.warn(logMessage);
+ }
+
RuntimeException e = LoadUtil.targetRuntimeException(t);
Printer.printError("Failed to load", e);
- if (this.context.options().testMode) {
- throw e;
- }
- } finally {
- this.stopThenShutdown();
+ LOG.error("Load failed with exception", e);
+
+ throw e;
}
+
return this.context.noError();
Review Comment:
‼️ **Critical: 异常处理逻辑变更可能导致错误被掩盖**
在 `load()` 方法中,ServerException 被捕获后只是记录日志,然后重新抛出。但在 catch 块中调用了
`this.context.occurredError()` 之后,仍然抛出异常,这可能导致:
1. 错误状态被标记,但错误本身仍在传播
2. `load()` 方法最后返回 `this.context.noError()`,但在异常路径中这行代码永远不会被执行
3. 与原有的返回值语义不一致(原代码注释说 'load() always returns true instead of
context.noError()')
**建议:**
- 明确异常处理策略:是捕获并返回false,还是让异常继续传播?
- 如果要改变返回值语义,需要更新相关调用方的逻辑
--
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]
