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

[gnokey] vm/qeval inconvenient parsing and non-existent erroring #2236

Open
leohhhn opened this issue May 28, 2024 · 6 comments
Open

[gnokey] vm/qeval inconvenient parsing and non-existent erroring #2236

leohhhn opened this issue May 28, 2024 · 6 comments
Labels
🐞 bug Something isn't working hacktoberfest This might be a good issue for a hacktoberfest participant to handle. help wanted Want to contribute? We recommend these issues.

Comments

@leohhhn
Copy link
Contributor

leohhhn commented May 28, 2024

Description

When I want to do a gnokey query vm/qeval, I would expect to be able to write a command like this:

gnokey query vm/qeval -remote https://rpc.gno.land:443 -data "gno.land/r/demo/wugnot BalanceOf("g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5")" 

However, the quotation marks need to be escaped (smaller issue), and the command needs to be formatted in a very weird way, with a newline in the middle, which is super inconvenient for CLI commands:

gnokey query vm/qeval -remote https://rpc.gno.land:443 -data "gno.land/r/demo/wugnot
BalanceOf(\"g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5\")" 

Also, if you do not do this, you will just get a 500 error and the transaction will break with no good response on why it happened:

❯ gnokey query vm/qeval -remote https://rpc.gno.land:443 -data "gno.land/r/demo/wugnot BalanceOf("g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5")" 
--= Error =--
Data: unable to call RPC method abci_query, invalid status code received, 500
Msg Traces:
    0  /Users/sasurai/Desktop/gno/gno/tm2/pkg/crypto/keys/client/query.go:111 - querying
--= /Error =--
@leohhhn leohhhn changed the title [gnokey] vm/qeval inconvenient parsing [gnokey] vm/qeval inconvenient parsing and non-existent erroring May 28, 2024
@leohhhn
Copy link
Contributor Author

leohhhn commented May 28, 2024

Same goes for vm/qrender - the following simply does not work, giving a 500 err message:

gnokey query vm/qrender --data "gno.land/r/demo/boards" -remote https://rpc.gno.land:443

Not sure if this is an error.

@leohhhn
Copy link
Contributor Author

leohhhn commented May 28, 2024

On top of everything, I think we definitely need to add a list of possible query arguments to gnokey query, as the only way to know them currently is the code/docs.

@linhpn99
Copy link
Contributor

Why do we return both the code and message in ResultABCIQuery in case of an error? This helps easily determine the internal state of the application and selectively filter information to be returned

@linhpn99
Copy link
Contributor

linhpn99 commented May 29, 2024

We could set the HTTP status code to always be OK, and only use the code from ResultABCIQuery

@Kouteki Kouteki added the 🐞 bug Something isn't working label Jun 7, 2024
@Kouteki Kouteki moved this from Triage to Todo in 🧙‍♂️gno.land core team Jun 7, 2024
@thehowl
Copy link
Member

thehowl commented Jul 1, 2024

We could set the HTTP status code to always be OK

Lol, no

Reminds me of the one time I added a pls200=1 querystring parameter to my API because I was using C# and didn't understand how exceptions worked (around 10 years ago)

Anyway, this bug seems to be mostly fixed for the gnokey --data inconvenience; maybe though we should still make the error messages show more information if available (and I think it is.)

@grepsuzette
Copy link
Contributor

grepsuzette commented Jul 4, 2024

We could set the HTTP status code to always be OK, and only use the code from ResultABCIQuery

Instead of always returning an OK status, we could have gnokey show the ResultABCIQuery data even upon 4xx and especially upon 5xx, as in #2422.

See PR #2501

@Kouteki Kouteki modified the milestone: 💡Someday/Maybe Oct 2, 2024
@Kouteki Kouteki added help wanted Want to contribute? We recommend these issues. and removed help wanted labels Oct 2, 2024
@wyhaines wyhaines added the hacktoberfest This might be a good issue for a hacktoberfest participant to handle. label Oct 9, 2024
@Kouteki Kouteki moved this from Todo to Backlog in 🧙‍♂️gno.land core team Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working hacktoberfest This might be a good issue for a hacktoberfest participant to handle. help wanted Want to contribute? We recommend these issues.
Projects
Status: Backlog
Development

No branches or pull requests

6 participants