moltin / js-sdk

JavaScript & Node.js SDKs for the Elastic Path Commerce Cloud eCommerce API
http://documentation.elasticpath.com/
MIT License
173 stars 77 forks source link

Deleting a product throws UnhandledPromiseRejectionWarning and breaks code. #105

Closed aravindanve closed 7 years ago

aravindanve commented 7 years ago

Hi,

When I call gateway.Products.Delete(UUID), my code breaks with UnhandledPromiseRejectionWarning. I can't seem to catch the error by .catch(e => console.log(e)) either.

I tested the API endpoint directly. That seems to work. Although, it returns an empty response, whereas, according to the docs, its supposed to return {type: 'product', id: UUID}. I think the sdk is trying to parse the empty response as JSON.

I suspect the code at src/utils/helpers.js line 42 is to blame. When I add a .catch() to the line like this: response.json().catch(e => ...).then() directly inside node_modules I'm able to catch this error:

{ Error
    at /Users/aravindanve/Work/temp/node_modules/node-fetch/lib/body.js:48:31
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
  name: 'FetchError',
  message: 'invalid json response body at https://api.moltin.com/v2/products/745468c9-c7a4-436d-aac7-5b4687f674f9 reason: Unexpected end of JSON input',
  type: 'invalid-json' }
ynnoj commented 7 years ago

Hey @aravindanve, thanks for raising this issue.

We need to do a better job handling an empty API response body, specifically with a 204 status. Although this conforms with JSON API spec, the fetch polyfill will throw an exception as there's no JSON in the response to parse in the parseJSON function 👇

https://github.com/moltin/js-sdk/blob/9df506a984409844da7cca9262402cd14c93c1bf/src/utils/helpers.js#L41-L48

I'll improve the parseJON function to handle an empty 204 response and cut a new patch release today.

aravindanve commented 7 years ago

@ynnoj awesome, thanks!

Also, you may need to update the docs here: https://moltin.api-docs.io/v2/products/delete-a-product

ynnoj commented 7 years ago

@aravindanve Noted, thanks!