[Bug][Zeta] SeaTunnelClient can not exit with error #9281

CosmosNi · 2025-05-07T01:06:43Z

SeaTunnelClient can not exit with error

close #9257

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

If any new Jar binary package adding in your PR, please add License Notice according
New License Guide
If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
If you are contributing the connector code, please check that the following files are updated:
1. Update plugin-mapping.properties and add new connector information in it
2. Update the pom file of seatunnel-dist
3. Add ci label in label-scope-conf
4. Add e2e testcase in seatunnel-e2e
5. Update connector plugin_config
Update the release-note.

Copilot

Pull Request Overview

This pull request fixes a bug where SeaTunnel components do not exit with an error by adding explicit error handling with System.exit(1) on fatal errors during Hazelcast initialization. Key changes include modifications in the server starter, Hazelcast client, and client command components to catch Errors and call System.exit(1).

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

File	Description
seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/SeaTunnelServerStarter.java	Added try-catch block around Hazelcast initialization to log and exit on fatal errors.
seatunnel-engine/seatunnel-engine-client/src/main/java/org/apache/seatunnel/engine/client/SeaTunnelHazelcastClient.java	Updated the constructor to catch Errors during client initialization and exit with an error code.
seatunnel-core/seatunnel-starter/src/main/java/org/apache/seatunnel/core/starter/seatunnel/command/ClientExecuteCommand.java	Wrapped Hazelcast instance creation in a try-catch block to handle fatal errors and exit properly.

Files not reviewed (1)

pom.xml: Language not supported

Copilot · 2025-05-07T01:50:45Z

...l-engine-server/src/main/java/org/apache/seatunnel/engine/server/SeaTunnelServerStarter.java

+        } catch (Error e) {
+            log.error("Fatal error occurred during Hazelcast server initialization", e);
+            System.exit(1);
+            throw e;


[nitpick] After calling System.exit(1), rethrowing the error is redundant since the JVM will terminate. Consider removing the rethrow for clarity unless it is required for testing purposes.

Suggested change

throw e;

Copilot · 2025-05-07T01:50:46Z

...engine-client/src/main/java/org/apache/seatunnel/engine/client/SeaTunnelHazelcastClient.java

+        } catch (Error e) {
+            log.error("Fatal error occurred during Hazelcast client initialization", e);
+            System.exit(1);
+            throw e;


[nitpick] The rethrow after System.exit(1) is generally unnecessary since the process will terminate. Consider removing or documenting this pattern if it serves a specific testing purpose.

Suggested change

throw e;

Copilot · 2025-05-07T01:50:46Z

.../src/main/java/org/apache/seatunnel/core/starter/seatunnel/command/ClientExecuteCommand.java

+        } catch (Error e) {
+            log.error("Fatal error occurred during Hazelcast instance initialization", e);
+            System.exit(1);
+            throw e;


[nitpick] Rethrowing the exception after calling System.exit(1) is redundant. Consider removing the rethrow to avoid unreachable code or adding a clarifying comment if needed for testing.

Suggested change

throw e;

// Note: The following line is unreachable due to System.exit(1).

// Uncomment for testing purposes if System.exit is mocked.

// throw e;

Hisoka-X · 2025-05-07T02:28:12Z

.../src/main/java/org/apache/seatunnel/core/starter/seatunnel/command/ClientExecuteCommand.java

+            return HazelcastInstanceFactory.newHazelcastInstance(
+                    seaTunnelConfig.getHazelcastConfig(),
+                    Thread.currentThread().getName(),
+                    new SeaTunnelNodeContext(seaTunnelConfig));


create hazelcast instance is one of potential crisis. So we should handle it in

seatunnel/seatunnel-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/SeaTunnel.java

Line 40 in 0b1122c

command.execute();

to catch all exist problem.

Hisoka-X · 2025-05-07T10:56:25Z

wating test case passes.

Hisoka-X · 2025-05-09T02:37:18Z

Hi @CosmosNi , please merge from dev to disable dead link check.

Hisoka-X · 2025-05-12T04:21:56Z

...l-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/SeaTunnel.java

            showConfigError(e);
+            System.exit(1);
            throw e;
-        } catch (Exception e) {
+        } catch (Exception | Error e) {
            showFatalError(e);
+            System.exit(1);


we should print the error statck before exit. Should we should update showConfigError and showFatalError

Hisoka-X · 2025-05-12T11:35:41Z

...l-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/SeaTunnel.java

@@ -40,9 +40,11 @@ public static <T extends CommandArgs> void run(Command<T> command) throws Comman
            command.execute();
        } catch (ConfigRuntimeException e) {
            showConfigError(e);
+            System.exit(1);
            throw e;


Suggested change

throw e;

Hisoka-X · 2025-05-12T11:35:48Z

...l-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/SeaTunnel.java

            showFatalError(e);
+            System.exit(1);
            throw e;


Suggested change

throw e;

Hisoka-X · 2025-05-12T11:37:25Z

...k-15-starter/src/test/java/org/apache/seatunnel/core/starter/flink/FlinkCommandArgsTest.java

+                                // Verify the exception message
+                                Assertions.assertEquals(
+                                        String.format(
+                                                "No configuration setting found for key '%s'",
+                                                PLUGIN_NAME.key()),
+                                        e.getCause().getMessage());
+                                // Re-throw to ensure the test fails if System.exit is not called
+                                throw e;


I believe the catch block will never reach. We can delete it.

Hisoka-X · 2025-05-12T11:39:27Z

...er/src/test/java/org/apache/seatunnel/core/starter/seatunnel/args/ClientCommandArgsTest.java

+                                SeaTunnel.run(clientCommandArgs.buildCommand());
+                            } catch (CommandExecuteException e) {
+                                // Verify the exception message
+                                Assertions.assertEquals(


Hisoka-X · 2025-05-12T11:39:32Z

...rk-3-starter/src/test/java/org/apache/seatunnel/core/starter/spark/SparkCommandArgsTest.java

+                                SeaTunnel.run(sparkCommandArgs.buildCommand());
+                            } catch (CommandExecuteException e) {
+                                // Verify the exception message
+                                Assertions.assertEquals(


Hisoka-X

LGTM if ci passes. Thanks @CosmosNi

hailin0 · 2025-05-13T12:41:16Z

Please retry ci

Hisoka-X · 2025-05-15T03:49:18Z

...use-e2e/src/test/java/org/apache/seatunnel/connectors/seatunnel/clickhouse/ClickhouseIT.java

+        // errors
+        //        Assertions.assertTrue(
+        //                execResult
+        //                        .getStderr()
+        //                        .contains(
+        //                                "ErrorCode:[API-11], ErrorDescription:[The sink table not
+        // exist]"));


In fact we still should get error msg from execResult. Please check where we can get the error msg now?

Hisoka-X · 2025-05-16T04:02:09Z

...tarter/src/test/java/org/apache/seatunnel/core/starter/seatunnel/SeaTunnelClientOOMTest.java

+public class SeaTunnelClientOOMTest {
+
+    @Test
+    public void testHazelcastOOMExitBehavior() throws Exception {


I found two test case are same. Any code missed update or we can keep one?

Hisoka-X

LGTM if ci passes.

[Bug][Zeta] SeaTunnelClient can not exit with error

6fda98f

github-actions bot added core SeaTunnel core module Zeta labels May 7, 2025

hailin0 requested a review from Copilot May 7, 2025 01:50

Copilot AI reviewed May 7, 2025

View reviewed changes

Hisoka-X reviewed May 7, 2025

View reviewed changes

CosmosNi added 2 commits May 7, 2025 12:43

[Bug][Zeta] SeaTunnelClient can not exit with error

fc8a4da

[Bug][Zeta] SeaTunnelClient can not exit with error

fe1c64e

github-actions bot removed the Zeta label May 7, 2025

CosmosNi added 6 commits May 8, 2025 09:00

Merge remote-tracking branch 'upstream/dev' into fix_exit_error

1c76422

[Bug][Zeta] SeaTunnelClient can not exit with error

c3e91e8

[Bug][Zeta] SeaTunnelClient can not exit with error

cf22c7c

Merge remote-tracking branch 'upstream/dev' into fix_exit_error

a1f9eb3

[Bug][Zeta] SeaTunnelClient can not exit with error

0610a4e

[Bug][Zeta] SeaTunnelClient can not exit with error

226e7f1

CosmosNi added 2 commits May 9, 2025 13:34

Merge remote-tracking branch 'upstream/dev' into fix_exit_error

17c160d

[Bug][Zeta] SeaTunnelClient can not exit with error

0b00eff

Hisoka-X reviewed May 12, 2025

View reviewed changes

CosmosNi added 2 commits May 12, 2025 13:37

[Bug][Zeta] SeaTunnelClient can not exit with error

58fb389

[Bug][Zeta] SeaTunnelClient can not exit with error

9312afd

Hisoka-X reviewed May 12, 2025

View reviewed changes

[Bug][Zeta] SeaTunnelClient can not exit with error

daa6c6c

Hisoka-X previously approved these changes May 13, 2025

View reviewed changes

github-actions bot added approved reviewed labels May 13, 2025

hailin0 previously approved these changes May 13, 2025

View reviewed changes

[Bug][Zeta] SeaTunnelClient can not exit with error

d978d14

CosmosNi dismissed stale reviews from hailin0 and Hisoka-X via d978d14 May 14, 2025 01:07

github-actions bot added e2e and removed approved reviewed labels May 14, 2025

hailin0 previously approved these changes May 15, 2025

View reviewed changes

github-actions bot added approved reviewed labels May 15, 2025

Hisoka-X requested changes May 15, 2025

View reviewed changes

github-actions bot removed approved reviewed labels May 15, 2025

[Bug][Zeta] SeaTunnelClient can not exit with error

ad9f560

CosmosNi dismissed hailin0’s stale review via ad9f560 May 16, 2025 00:55

CosmosNi added 2 commits May 16, 2025 08:57

Merge remote-tracking branch 'upstream/dev' into fix_exit_error

0a45694

[Bug][Zeta] SeaTunnelClient can not exit with error

f317b81

github-actions bot removed the e2e label May 16, 2025

Hisoka-X reviewed May 16, 2025

View reviewed changes

[Bug][Zeta] SeaTunnelClient can not exit with error

a55dc87

Hisoka-X approved these changes May 16, 2025

View reviewed changes

github-actions bot added approved reviewed labels May 16, 2025

hailin0 approved these changes May 17, 2025

View reviewed changes

hailin0 merged commit d023cf2 into apache:dev May 17, 2025
5 checks passed

-            throw e;
+            // Note: The following line is unreachable due to System.exit(1).
+            // Uncomment for testing purposes if System.exit is mocked.
+            // throw e;

[Bug][Zeta] SeaTunnelClient can not exit with error #9281

[Bug][Zeta] SeaTunnelClient can not exit with error #9281

Uh oh!

Conversation

CosmosNi commented May 7, 2025 • edited by hailin0 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hisoka-X commented May 7, 2025

Uh oh!

Hisoka-X commented May 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hisoka-X left a comment

Choose a reason for hiding this comment

Uh oh!

hailin0 commented May 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hisoka-X left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

CosmosNi commented May 7, 2025 •

edited by hailin0

Loading