Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix error with work_cancel RPC request #2916

Merged
merged 2 commits into from
Sep 11, 2020

Conversation

koczadly
Copy link
Contributor

@koczadly koczadly commented Sep 7, 2020

work_cancel currently returns no response data when successful, which results in an "Empty response" error being returned. Appears to have been broken from PR #1032.

This PR adds:

  • Return {"success": ""} as the response for work_cancel
  • Add nano::error_rpc::empty_response error code, which is returned for empty responses (previously only an error string was passed, but without an error code).
  • Update existing tests which expected an empty response error

Unfortunately I've been unable to test this as I couldn't get boost to link up with VS correctly. Not sure what I'm doing wrong, so you'll have to rely on the CI results.

clemahieu
clemahieu previously approved these changes Sep 7, 2020
@guilhermelawless
Copy link
Contributor

guilhermelawless commented Sep 8, 2020

Return {"success": ""} as the response for work_generate

@koczadly did you mean work_cancel ?

Thanks for the fix, we require however that commits are GPG-signed. If you don't mind we can alternatively commit the patch ourselves with you as co-author.

…successfully, and implement nano::error_rpc::empty_response error code to catch future errors.
@koczadly
Copy link
Contributor Author

koczadly commented Sep 8, 2020

@guilhermelawless

Yeah sorry, I meant work_cancel. I've squashed and re-committed it with a signature, apologies.

@guilhermelawless guilhermelawless added rpc Changes related to Remote Procedure Calls documentation This item indicates the need for or supplies updated or expanded documentation labels Sep 8, 2020
@guilhermelawless guilhermelawless added this to the V22.0 milestone Sep 8, 2020
@guilhermelawless guilhermelawless added interface Change to node APIs (separate label) which impacts the interface, integrations impacted. bug labels Sep 8, 2020
Copy link
Contributor

@guilhermelawless guilhermelawless left a comment

Choose a reason for hiding this comment

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

Just some minor suggestions

nano/core_test/fakes/work_peer.hpp Show resolved Hide resolved
nano/rpc_test/rpc.cpp Show resolved Hide resolved
@clemahieu clemahieu merged commit 185de4e into nanocurrency:develop Sep 11, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug documentation This item indicates the need for or supplies updated or expanded documentation interface Change to node APIs (separate label) which impacts the interface, integrations impacted. rpc Changes related to Remote Procedure Calls
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants