Skip to content

Conversation

@briceyan
Copy link
Contributor

@briceyan briceyan commented Nov 29, 2024

This PR tries to address two issues related to subcommand add:

  1. --contract-name flag ignored: When the --contract-name flag is specified in the command line, the provided value is ignored, and the user is prompted for input

  2. Improved handling of --start-block flag: When the --start-block flag is provided, its value is correctly applied, but an unnecessary asynchronous call was still being made. This resulted in execution hanging due to an unhandled promise. The fix eliminates the redundant asynchronous call

@changeset-bot
Copy link

changeset-bot bot commented Nov 29, 2024

🦋 Changeset detected

Latest commit: 3e4a299

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphprotocol/graph-cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@0237h
Copy link
Collaborator

0237h commented Nov 29, 2024

Hey @briceyan, thanks for the PR. Would you mind adding a description of the fix ?

I understand it fixes an issue with the add command not respecting flag parameters and I could reproduce the issue, appreciate the fix.

@briceyan
Copy link
Contributor Author

briceyan commented Nov 30, 2024

I initially suspected that the process hanging after completing the code generation was caused by some leftover async operations, but it now seems likely that prompt.ask is the reason for the hang. Unfortunately, I don’t have enough time to investigate this further at the moment—apologies for the incomplete diagnosis.

Below is the output when I ran the command with why-is-node-running:

...

+ why-is-node-running /Users/brice/Devel/graph-tooling/packages/cli/bin/run add --abi /Users/brice/EverythingFun/v1-core/out/ISet.sol/ISet.json --merge-entities --contract-name Set12 --start-block 1 0x0000000000000000000000000000000000000000
probing program /Users/brice/Devel/graph-tooling/packages/cli/bin/run
kill -SIGUSR1 19346 for logging
(node:19346) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
✖ Failed to fetch Contract Name: Failed to fetch contract source code: Contract source code not verified
✔ Contract Name · Set12
✔ Running codegen
There are 17 handle(s) keeping the process running.

# TTYWRAP
/Users/brice/Library/pnpm/global/5/.pnpm/[email protected]/node_modules/why-is-node-running/include.js:6 - console.log('kill -SIGUSR1', process.pid, 'for logging')

# TTYWRAP
(unknown stack trace)

# TTYWRAP
/Users/brice/Devel/graph-tooling/node_modules/.pnpm/[email protected]/node_modules/password-prompt/index.js:3 - const stdin = process.stdin

# FILEHANDLE
(unknown stack trace)

# FILEHANDLE
(unknown stack trace)

...

@0237h
Copy link
Collaborator

0237h commented Dec 3, 2024

@briceyan Not sure I fully understand, are you running into some other issue even after the fix of this PR ?

Copy link
Collaborator

@YaroShkvorets YaroShkvorets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, it was indeed a bug. See my comments.

@YaroShkvorets YaroShkvorets self-requested a review December 16, 2024 03:25
@YaroShkvorets YaroShkvorets merged commit 5900f09 into graphprotocol:main Dec 16, 2024
9 checks passed
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.

3 participants