Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Multiple try/catch+zombie fixes #97

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

hlolli
Copy link

@hlolli hlolli commented Jul 19, 2021

Changes

  • remove unneccecary async try-catches
  • Improve randomized NODE selection
  • Prevent zombie processes with proper promisified recursion

Rebased over #96, I'll re-rebase after that gets merged.

But the problem I'm hitting is stuff like this, and for parts which can fail, should fail. The stack gets usually lost in async functions (not in Promises) and a series of them, makes debugging very tedious.

yarn start
yarn run v1.22.10
$ npm run migrate:latest && node dist/src/Gateway.js

> @arweave/gateway@1.0.0 migrate:latest
> knex migrate:latest

Requiring external module ts-node/register
Already up to date
info: [app] started on http://localhost:3000
info: [database] starting sync, parallelization is set to 2
node:internal/process/promises:246
          triggerUncaughtException(err, true /* fromPromise */);
          ^

Error: connect ECONNREFUSED 163.47.11.64:1984
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1133:16) {
  errno: -61,
  code: 'ECONNREFUSED',
  syscall: 'connect',
  address: '163.47.11.64',
  port: 1984,
  response: undefined
}
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

With this change I'm able to get somewhat started

yarn start
yarn run v1.22.10
$ npm run migrate:latest && node dist/src/Gateway.js

> @arweave/gateway@1.0.0 migrate:latest
> knex migrate:latest

Requiring external module ts-node/register
Already up to date
info: [app] started on http://localhost:3000
info: [database] starting sync, parallelization is set to 2
info: [database] syncing from block 0 to 730868
1/730868 blocks synced [                                                                                                    ] 0% 0.0sinfo: [snapshot] could not retrieve block at height 0, retrying
5/730868 blocks synced [                                                                                               ] 0% 343944.1sinfo: [snapshot] could not retrieve block at height 5, retrying
info: [snapshot] could not retrieve block at height 5, retrying

Prevent zombie processes with proper promisified recursion
@hlolli hlolli changed the title remove unneccecary async try-catches Multiple try/catch+zombie fixes Jul 19, 2021
@TheLoneRonin
Copy link
Contributor

TheLoneRonin commented Sep 10, 2021

Hey @hlolli,

I think the idea you're going for here on this PR is good and makes sense.
However, this codebase is going to be completely rehauled this coming week.

Are you available to chat about this source and Vartex sometime soon?

You can actually review: https://github.com/ArweaveTeam/gateway/tree/v1.0.1
To see some of the changes coming through.

Thanks.

@TheLoneRonin
Copy link
Contributor

I'm also probably going to migrate the improvements to try/catch queries to that branch. Unless you want to open a PR to v1.0.1 with that specifically.

Thank you :)

@hlolli
Copy link
Author

hlolli commented Sep 10, 2021

@TheLoneRonin

The issue at the time was that errors were taking place and they weren't visible. I have since discovered the culprit, which is that streams need to be handled with .on("error", cb) since the callsite where the error is thrown isn't always at the consumer side where we are trying to catch the error. Irritating when this caveat isn't known.

Let's chat soon!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants