Code implemented to handle/disable auto-shutdown of idle connections code can crash cloud nodes

[jeremygiberson]

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch @ulixee/hero-core@2.0.0-alpha.25 for the project I'm working on.

Work has previously been done to disable auto-shutdown of idle connections here: https://github.com/ulixee/hero/pull/199 and the method introduced to check for idle timeout ms fails in the edge cases when the hero connection has already disconnected due to an error.

I observed the following in the logs:

[0] 2023-11-09T07:24:56.132Z ERROR [hero-core/index] UnhandledError(fatal) {
--
[0]   origin: 'uncaughtException',
[0]   context: {},
[0]   sessionId: null,
[0]   sessionName: undefined
[0] } TypeError: Cannot read properties of null (reading 'clearIdleConnectionsAfterMillis')
[0]     at ConnectionToHeroClient.get autoShutdownMillis [as autoShutdownMillis] (/app/ulixee/node_modules/core/connections/ConnectionToHeroClient.ts:37:22)
[0]     at ConnectionToHeroClient.disconnectIfInactive (/app/ulixee/node_modules/core/connections/ConnectionToHeroClient.ts:262:33)
[0]     at listOnTimeout (node:internal/timers:569:17)
[0]     at process.processTimers (node:internal/timers:512:7)
[0] 2023-11-09T07:24:56.132Z STATS [commons/lib/ShutdownHandler] ShutdownHandler.onSignal { signal: 'exit', context: {} }
[0] 2023-11-09T07:24:56.133Z STATS [commons/lib/ShutdownHandler] ShutdownHandler.execute {
[0]   signal: 'exit',
[0]   fn: 'function () { [native code] }',
[0]   callsite: 'at new CloudNode (/app/ulixee/cloud/main/lib/CloudNode.ts:148:21)',
[0]   context: {}
[0] }

I traced the cause to this line where this.core is set to null on disconnect: https://github.com/ulixee/hero/blob/main/core/connections/ConnectionToHeroClient.ts#L167

The checkForAutoshutdown is bound to the close event here https://github.com/ulixee/hero/blob/main/core/connections/ConnectionToHeroClient.ts#L202

Which then blows up when trying to retrieve the timeout ms from the core object here https://github.com/ulixee/hero/blob/main/core/connections/ConnectionToHeroClient.ts#L202

I'm mitigating this issue by adding the clearIdleConnectionsAfterMillis property to the connectionToHero object that is copied from the core instance during the constructor, that way the value is still accessible after this.core is cleared out on disconnect.

Here is the diff that solved my problem:

diff --git a/node_modules/@ulixee/hero-core/connections/ConnectionToHeroClient.js b/node_modules/@ulixee/hero-core/connections/ConnectionToHeroClient.js
index 54d1611..d67c16a 100644
--- a/node_modules/@ulixee/hero-core/connections/ConnectionToHeroClient.js
+++ b/node_modules/@ulixee/hero-core/connections/ConnectionToHeroClient.js
@@ -20,6 +20,7 @@ class ConnectionToHeroClient extends eventUtils_1.TypedEventEmitter {
         super();
         this.transport = transport;
         this.core = core;
+        this.clearIdleConnectionsAfterMillis = core.clearIdleConnectionsAfterMillis;
         this.sessionIdToRemoteEvents = new Map();
         this.activeCommandMessageIds = new Set();
         transport.on('message', message => this.handleRequest(message));
@@ -29,7 +30,7 @@ class ConnectionToHeroClient extends eventUtils_1.TypedEventEmitter {
         this.disconnectIfInactive = this.disconnectIfInactive.bind(this);
     }
     get autoShutdownMillis() {
-        return this.core.clearIdleConnectionsAfterMillis;
+        return this.clearIdleConnectionsAfterMillis;
     }
     ///////  CORE CONNECTION  /////////////////////////////////////////////////////////////////////////////////////
     async handleRequest(payload) {

This issue body was partially generated by patch-package.

[blakebyrnes]

Thanks! Will apply a fix on next release.