Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Fix state bad head Internal server error [STL-1345]. #1817

Closed

Conversation

shresthichauhan
Copy link

@shresthichauhan shresthichauhan commented Aug 9, 2018

Return JSON response with error code 60. Head id added
to list_state handler function in rest api route handlers.

Signed-off-by: shresthichauhan shresthix.chauhan@intel.com

@shresthichauhan shresthichauhan changed the title Fix for STL-1345 Fix state bad head Internal server error [STL-1345]. Aug 10, 2018
@dcmiddle dcmiddle self-requested a review August 20, 2018 04:28
@dcmiddle dcmiddle self-assigned this Aug 20, 2018
Copy link
Contributor

@dcmiddle dcmiddle 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 fix. Can you please copy the actual result returned with this additional line as a comment in this pull request? i.e. demonstrate that the bug is resolved.

Also would you mind adding a unit test case in this PR so we can catch future regressions?

@shresthichauhan
Copy link
Author

Thank you so much for your review comments.

Please find actual result returned with this additional line. This is to demonstrate bug is resolved.

after_fix_json_response_60

As we know state head is block ID that is 128 character hex-strings. So fix will return JSON response with error code 60 for bad Head parameter.

Sure will add a unit test case in this PR and will inform you about the same.

@@ -262,7 +262,7 @@ def __init__(
paging: Paging info and nav, like total resources and a next link
"""
paging_controls = self._get_paging_controls(request)

head_id=self._get_head_id(request)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be pushed down into the _head_to_root function - this particular error can occur in the fetch_state function as well.

Likewise, as that head_id is unused, I suspect the build is failing due to lint

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for suggesting _head_to_root function.

PR specific to list_state function (for endpoint '/state'). Please refer "start_rest_api" in rest_api.py, fetch_state for '/state/{address}' endpoint.

As per my understanding, ClientStateListRequest is only function where head_id not pass as parameter in comparison to other functions (ClientTransactionListRequest, ClientBatchListRequest and ClientBlockListRequest).

message ClientStateListRequest {
string state_root = 1;
string address = 3;
ClientPagingControls paging = 4;
repeated ClientSortControls sorting = 5;
}

message ClientBlockListRequest {
string head_id = 1;
repeated string block_ids = 2;
ClientPagingControls paging = 3;
repeated ClientSortControls sorting = 4;
}

Adding head_id in ClientStateListRequest, impact many other functionality. Please suggest.

clientblocklistrequest

Removed lint Error (Unused variable 'head_id')
Return JSON response with error code 60.Head id
added to list_state handler function in rest
api route handlers.

Signed-off-by: shresthichauhan <shresthix.chauhan@intel.com>
@@ -262,7 +262,7 @@ def __init__(
paging: Paging info and nav, like total resources and a next link
"""
paging_controls = self._get_paging_controls(request)

self._get_head_id(request)
Copy link
Contributor

Choose a reason for hiding this comment

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

The return value should be passed to _head_to_root. This now hides the fact that you're calling that to validate the input. A future developer may come along and delete that line, thinking its extraneous.

@dcmiddle dcmiddle closed this Dec 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants