shrimpy-node icon indicating copy to clipboard operation
shrimpy-node copied to clipboard

Fix returns

Open Fi1osof opened this issue 6 years ago • 6 comments

Note: async functions should return pure call, not await (for .catch()).

Fi1osof avatar Sep 03 '19 15:09 Fi1osof

Related to #1

Fi1osof avatar Sep 03 '19 15:09 Fi1osof

What is the purpose of this PR? You mention it's "for .catch()", but these methods already propagate errors via "await"

shrimpy-dev avatar Sep 14 '19 18:09 shrimpy-dev

@shrimpy-dev so you should return return await this._callEndpoint<any>(endpoint, 'POST', null, true); instead await this._callEndpoint<any>(endpoint, 'POST', null, true);

await this._callEndpoint<any>(endpoint, 'POST', null, true); does not return anything.

here you do return, but here not.

Fi1osof avatar Sep 14 '19 21:09 Fi1osof

Right, we aren't always expecting data. Using your examples, you expect a list of strings back from getIpWhitelistAddresses, but just want to know if the operation was successful for deleteAccount. Because the operation not throwing (i.e. .then() vs .catch()) is enough to determine success, the {"success": true} object doesn't need to be returned to the caller.

Now that you mention it, I see a number of places where we should replace return await with simply await.

shrimpy-dev avatar Sep 14 '19 23:09 shrimpy-dev

Now that you mention it, I see a number of places where we should replace return await with simply await.

@shrimpy-dev, not "replace return await with simply await.", need replace "simply await" with "return ..." or "return await ...".

See your official API documentation, for example https://developers.shrimpy.io/docs/#naming-a-user Response: { "success": true }

And see code: https://github.com/shrimpy-dev/shrimpy-node/blob/e8bf8415a04f6c72e022478d6d13e703aec17ba5/lib/client/shrimpy-api-client.ts#L239-L245 This code does not responsing { "success": true }, it's responsing nothing.

Fi1osof avatar Sep 15 '19 03:09 Fi1osof

Right, the docs cover the request/response data. Just because { "success": true }, is sent from the server, doesn't mean it needs to be sent to the caller of the client method (i.e. client.setUserName).

In the docs you linked, you can see the expected usage is await client.setUserName(...), not result = await client.setUserName(...). The same is true in this library's README.md file.

Is there a reason that you want the { "success": true } object to be returned from the client call? What do you plan on doing with it?

shrimpy-dev avatar Sep 17 '19 00:09 shrimpy-dev