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

APIs missing error when querying using invalid proposalId #3078

Closed
4 tasks
sabau opened this issue Dec 11, 2018 · 5 comments
Closed
4 tasks

APIs missing error when querying using invalid proposalId #3078

sabau opened this issue Dec 11, 2018 · 5 comments
Assignees
Labels
S:needs more info This bug can't be addressed until more information is provided by the reporter. T:Bug T: UX

Comments

@sabau
Copy link
Contributor

sabau commented Dec 11, 2018

Summary of Bug

I tried to query the SDK using a non existent proposalId, and the reply was null instead of the proper error body with message.

Steps to Reproduce


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alessio
Copy link
Contributor

alessio commented Dec 14, 2018

gaiacli works as expected:

alessio@bangalter:~/.../cosmos/cosmos-sdk$ gaiacli query gov votes --chain-id=test-chain-P0ajzL 3
ERROR: Failed to fetch proposal-id 3: {"codespace":"GOV","code":1,"message":"Unknown proposal with id 3"}

I guess the REST endpoint needs some love here.

@alessio
Copy link
Contributor

alessio commented Dec 14, 2018

Reproduced via test case:

diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go
index 164edb0f..82367e89 100644
--- a/client/lcd/lcd_test.go
+++ b/client/lcd/lcd_test.go
@@ -740,4 +740,8 @@ func TestProposalsQuery(t *testing.T) {
        require.Len(t, votes, 2)
        require.True(t, addrs[0].String() == votes[0].Voter.String() || addrs[0].String() == votes[1].Voter.String())
        require.True(t, addrs[1].String() == votes[0].Voter.String() || addrs[1].String() == votes[1].Voter.String())
+
+       // Test query votes on non-existent proposal
+       votes = getVotes(t, port, 99)
+       require.Len(t, votes, 0)
 }

The test pass, no error is returned. CC'ing who touched the code last: @sunnya97
Was return no error a design choice? I'd be inclined to think that 0 votes should be returned if the proposal exists but got no votes, whilst an error should be returned if there's no such proposal at all. Thoughts?

@alexanderbez
Copy link
Contributor

alexanderbez commented Dec 14, 2018

I think this overlaps with #3091. In that PR, I now return an error IF the proposal is inactive (votes/deposits removed from state). BUT, if the proposal is still active and there are no votes/deposits, it still returns null.

@alessio
Copy link
Contributor

alessio commented Dec 18, 2018

This should be fixed in ff6c2f2.
@sabau Would you mind to try reproduce the issue with latest develop please?

@alessio alessio added the S:needs more info This bug can't be addressed until more information is provided by the reporter. label Dec 18, 2018
@sabau
Copy link
Contributor Author

sabau commented Mar 27, 2019

Work nicely now

@sabau sabau closed this as completed Mar 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:needs more info This bug can't be addressed until more information is provided by the reporter. T:Bug T: UX
Projects
None yet
Development

No branches or pull requests

4 participants