Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

grisha87
Copy link
Contributor

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);

@grisha87 grisha87 requested a review from SewerynKras July 31, 2024 13:04
@grisha87 grisha87 merged commit 5adb483 into beta Jul 31, 2024
8 checks passed
@grisha87 grisha87 deleted the bugfix/missing-gftp-blocks-disconnect branch July 31, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants