Correctly initialise gRPCurl text area

[gerektoolhy]

Description

#241 introduced feature where a grpcurl command would be rendered. The grpcurl could then be copied and run in terminal repeatedly.

Issue

After some use, I have noticed a small bug where on initial page load, the grpcurl would contain an incorrectly serialized payload [Object object]. Expectation is that the payload should contain a valid JSON data. At a minimum this has to be an empty JSON object {}, or anything that has been entered either from the first tab, or manually in the Request box.

Root cause

The issue is introduced by line https://github.com/fullstorydev/grpcui/blob/master/internal/resources/webform/webform.js#L158 which calls, which calls updateCurlCommand(requestObj);. The problem with this line is that requestObj is not a string parameter. This object ends up being concatenated to a string, and gets translated to Object object.

To trace this issue down I have instrumented updateCurlCommand() function and logged out how many times it was called and the stack trace along with it. It turned out that on page load this function was called twice. Surprisingly, the first call contained the correct JSON value, but the second not.

The first call was made from updateJsonRequest, which converted request object to string and passed that to updateCurlCommand.

Fix

It turns out, that the second call to updateCurlCommand is not needed, and thus can be removed.

After fix the grpcurl textbox correctly contains a valid JSON as data parameter:

grpcurl -plaintext -d '{
  "diffIds": []
}' localhost:9011 <host_redacted>

[gerektoolhy]

FYI @dragonsinth

[TilGP]

Was able to replicate the issue and verify that the fix works

[gerektoolhy]

@TilGP anything else needs for this to be merged?

[TilGP]

@TilGP anything else needs for this to be merged?

Apparently a review from someone with write access, which I am not. (I'm not affiliated with fullstorydev either). I just reviewed it with the hope, that it will bring some attention from the maintainers to this PR.

[gerektoolhy]

@TilGP got it, I wrongly assumed that you were. Thanks for confirming the fix though.

FYI @dragonsinth @jhump

[jhump]

LGTM. Though I also do not have write access to this repo 😂. (I left FullStory a couple of years ago.)

@dragonsinth, @gpassini, could one of you two take a look?

[dragonsinth]

I'm not sure this is the best fix. I think the underlying problem is that updateJSONRequest takes an object/value while updateCurlCommand wants a string. I'm not even sure we want to use the same format... updateJSONRequest internally uses JSON.stringify(req, null, 2) while I think updateCurlCommand should internally use plain JSON.stringify(req).

[dragonsinth]

this part actually seems fine since updateJSONRequest calls this internally

[gerektoolhy]

@dragonsinth can you clarify please. The call to updateCurlCommand is not required here, since updateJSONRequest will do that. So removing this call actually fixes the issue.

[gerektoolhy]

I'm not sure this is the best fix. I think the underlying problem is that updateJSONRequest takes an object/value while updateCurlCommand wants a string. I'm not even sure we want to use the same format... updateJSONRequest internally uses JSON.stringify(req, null, 2) while I think updateCurlCommand should internally use plain JSON.stringify(req).

updateJSONRequest is calling updateCurlCommand, so there is no need first call updateJSONRequest. Removing the call to updateCurlCommand fixes the issue.