golemfactory / golem-js

TypeScript + NodeJS API for Golem
https://docs.golem.network/docs/creators/javascript
GNU Lesser General Public License v3.0
29 stars 16 forks source link

fix: fixed the issue blocking GolemNetwork.disconnecte because of missing gftp #1040

Closed grisha87 closed 1 month ago

grisha87 commented 1 month ago

The issue

In cases when the requestor does not have gftp installed (you can move the file to a different location to test this), glm.disconnect() was executed quickly and the script was not closing at all. Using wtfnode showed that after glm.connect() fails due to the missing gftp, some connections to yagna were open:

[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 1 (tty) (stdio)
  - fd 2 (tty) (stdio)
- Sockets:
  - 127.0.0.1:57112 -> 127.0.0.1:7465
  - 127.0.0.1:57142 -> 127.0.0.1:7465
  - 127.0.0.1:43576 -> 127.0.0.1:7465
- Timers:
  - (10000 ~ 10 s) (anonymous) @ /home/ggodlewski/proj/golem/golem-js/tmp/broken-gftp.ts:2

The solution

This PR removes the redundant isConnected check during the disconnect method execution, allowing the code to actually close the yagna connections. This way the V8 event loop can drain from work and the program can exit normally.

This does not affect negatively the indempotency of the disconnect method at all:

import {GolemNetwork} from "../src";

(async () => {
  const glm = new GolemNetwork();

  try {
    await glm.connect();
  } catch (err) {
    console.error(err);
  } finally {
    await glm.disconnect();
    await glm.disconnect();
    await glm.disconnect();
    console.log("Disconnected");
  }
})().catch(console.error);