From 25fef3d8d4682456b2db1571e3657d9144275dd7 Mon Sep 17 00:00:00 2001
From: Denys Otrishko <shishugi@gmail.com>
Date: Mon, 9 Jul 2018 01:44:27 +0300
Subject: [PATCH] workers: fix invalid exit code in parent upon uncaught
 exception

Now worker.on('exit') reports correct exit code (1) if worker has exited
with uncaught exception.

Fixes: https://github.com/nodejs/node/issues/21707

PR-URL: https://github.com/nodejs/node/pull/21713
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 lib/internal/worker.js                        |   3 +
 test/parallel/test-worker-exit-code.js        | 130 ++++++++++++++++++
 .../test-worker-uncaught-exception-async.js   |   4 +
 .../test-worker-uncaught-exception.js         |   4 +
 4 files changed, 141 insertions(+)
 create mode 100644 test/parallel/test-worker-exit-code.js

diff --git a/lib/internal/worker.js b/lib/internal/worker.js
index bcc864b5b8b330..83389d204d285f 100644
--- a/lib/internal/worker.js
+++ b/lib/internal/worker.js
@@ -454,6 +454,9 @@ function setupChild(evalScript) {
     debug(`[${threadId}] fatal exception caught = ${caught}`);
 
     if (!caught) {
+      // set correct code (uncaughtException) for [kOnExit](code) handler
+      process.exitCode = 1;
+
       let serialized;
       try {
         serialized = serializeError(error);
diff --git a/test/parallel/test-worker-exit-code.js b/test/parallel/test-worker-exit-code.js
new file mode 100644
index 00000000000000..bb47e1cece7a62
--- /dev/null
+++ b/test/parallel/test-worker-exit-code.js
@@ -0,0 +1,130 @@
+// Flags: --experimental-worker
+'use strict';
+const common = require('../common');
+
+// This test checks that Worker has correct exit codes on parent side
+// in multiple situations.
+
+const assert = require('assert');
+const worker = require('worker_threads');
+const { Worker, isMainThread, parentPort } = worker;
+
+if (isMainThread) {
+  parent();
+} else {
+  if (!parentPort) {
+    console.error('Parent port must not be null');
+    process.exit(100);
+    return;
+  }
+  parentPort.once('message', (msg) => {
+    switch (msg) {
+      case 'child1':
+        return child1();
+      case 'child2':
+        return child2();
+      case 'child3':
+        return child3();
+      case 'child4':
+        return child4();
+      case 'child5':
+        return child5();
+      case 'child6':
+        return child6();
+      case 'child7':
+        return child7();
+      default:
+        throw new Error('invalid');
+    }
+  });
+}
+
+function child1() {
+  process.exitCode = 42;
+  process.on('exit', (code) => {
+    assert.strictEqual(code, 42);
+  });
+}
+
+function child2() {
+  process.exitCode = 99;
+  process.on('exit', (code) => {
+    assert.strictEqual(code, 42);
+  });
+  process.exit(42);
+}
+
+function child3() {
+  process.exitCode = 99;
+  process.on('exit', (code) => {
+    assert.strictEqual(code, 0);
+  });
+  process.exit(0);
+}
+
+function child4() {
+  process.exitCode = 99;
+  process.on('exit', (code) => {
+    // cannot use assert because it will be uncaughtException -> 1 exit code
+    // that will render this test useless
+    if (code !== 1) {
+      console.error('wrong code! expected 1 for uncaughtException');
+      process.exit(99);
+    }
+  });
+  throw new Error('ok');
+}
+
+function child5() {
+  process.exitCode = 95;
+  process.on('exit', (code) => {
+    assert.strictEqual(code, 95);
+    process.exitCode = 99;
+  });
+}
+
+function child6() {
+  process.on('exit', (code) => {
+    assert.strictEqual(code, 0);
+  });
+  process.on('uncaughtException', common.mustCall(() => {
+    // handle
+  }));
+  throw new Error('ok');
+}
+
+function child7() {
+  process.on('exit', (code) => {
+    assert.strictEqual(code, 97);
+  });
+  process.on('uncaughtException', common.mustCall(() => {
+    process.exitCode = 97;
+  }));
+  throw new Error('ok');
+}
+
+function parent() {
+  const test = (arg, exit, error = null) => {
+    const w = new Worker(__filename);
+    w.on('exit', common.mustCall((code) => {
+      assert.strictEqual(
+        code, exit,
+        `wrong exit for ${arg}\nexpected:${exit} but got:${code}`);
+      console.log(`ok - ${arg} exited with ${exit}`);
+    }));
+    if (error) {
+      w.on('error', common.mustCall((err) => {
+        assert(error.test(err));
+      }));
+    }
+    w.postMessage(arg);
+  };
+
+  test('child1', 42);
+  test('child2', 42);
+  test('child3', 0);
+  test('child4', 1, /^Error: ok$/);
+  test('child5', 99);
+  test('child6', 0);
+  test('child7', 97);
+}
diff --git a/test/parallel/test-worker-uncaught-exception-async.js b/test/parallel/test-worker-uncaught-exception-async.js
index c3f0c8dec59f09..f820976c11ebcd 100644
--- a/test/parallel/test-worker-uncaught-exception-async.js
+++ b/test/parallel/test-worker-uncaught-exception-async.js
@@ -12,6 +12,10 @@ if (!process.env.HAS_STARTED_WORKER) {
   w.on('error', common.mustCall((err) => {
     assert(/^Error: foo$/.test(err));
   }));
+  w.on('exit', common.mustCall((code) => {
+    // uncaughtException is code 1
+    assert.strictEqual(code, 1);
+  }));
 } else {
   setImmediate(() => {
     throw new Error('foo');
diff --git a/test/parallel/test-worker-uncaught-exception.js b/test/parallel/test-worker-uncaught-exception.js
index b7d9f8a2928ec1..67b861e22619aa 100644
--- a/test/parallel/test-worker-uncaught-exception.js
+++ b/test/parallel/test-worker-uncaught-exception.js
@@ -12,6 +12,10 @@ if (!process.env.HAS_STARTED_WORKER) {
   w.on('error', common.mustCall((err) => {
     assert(/^Error: foo$/.test(err));
   }));
+  w.on('exit', common.mustCall((code) => {
+    // uncaughtException is code 1
+    assert.strictEqual(code, 1);
+  }));
 } else {
   throw new Error('foo');
 }