t3chnoboy / amazon-product-api

:credit_card: Amazon Product Advertising API client
365 stars 104 forks source link

Return Product API Errors from the Request #31

Closed FdezRomero closed 8 years ago

FdezRomero commented 8 years ago

Hi there!

I was annoyed that I wasn't getting errors from the API as they are shown at the Product API Scratchpad and added a few lines so the module can return these and know exactly what happened (now you can only see there are no Items in the result).

So API errors inside the Request node like this one:

<Errors>
  <Error>
    <Code>AWS.ECommerceService.ItemNotAccessible</Code>
    <Message>This item is not accessible through the Product Advertising API.</Message>
  </Error>
</Errors>

Are being returned as errors in the callback/promise methods in JSON:

[{
  "Error":
    [{
      "Code": ["AWS.ECommerceService.ItemNotAccessible"],
      "Message": ["This item is not accessible through the Product Advertising API."]
    }]
}]

Let me know if there's anything wrong with this or missing.

masterT commented 8 years ago

It looks good!

FdezRomero commented 8 years ago

Thanks for merging @masterT, the Travis CI test failed though.

BTW: I took the liberty of updating the package version number to 0.3.5 as I saw other previous changes. It would be good to publish this to NPM as well.

masterT commented 8 years ago

Yep, I am looking to fix it now.

masterT commented 8 years ago

I looked up the Amazon Error Docs and the AWS.ECommerceService.ItemNotAccessible is related to the method ItemLookup, so I will only check if there is an error when doing this method.

masterT commented 8 years ago

@FdezRomero are you sure you want to return the results as well, beside the error?

FdezRomero commented 8 years ago

Not sure about others methods really, but the AWS.ECommerceService.ItemNotAccessible was just an example. I've seen other errors when using ItemLookup and if Amazon had to return an error in other methods, this would be how they would return it, so I made it method-independent.

Regarding wether to return the results or not when using callbacks, I chose the conservative option of providing both and letting the developer choose what to do. From the docs, you should first check the existence of an error anyway. But it's true that the behaviour is different for promises, when you would just get the error in the .catch() and no results. So the choice is freedom vs. consistency, but I felt I was no one to make that decision ;-) I'm sure you can make a more informed decision.

masterT commented 8 years ago

All right! It makes sense!

masterT commented 8 years ago

This is an response when a make a request that throw an AWS.ECommerceService.ItemNotAccessible. There is no Items.Item in the response, so when using callback, it will pass only the error.

{ '$': { xmlns: 'http://webservices.amazon.com/AWSECommerceService/2011-08-01' },
  OperationRequest:
   [ { RequestId: [Object],
       Arguments: [Object],
       RequestProcessingTime: [Object] } ],
  Items: [ { Request: [Object] } ] }
FdezRomero commented 8 years ago

If that is what the API returns, that's what a developer would expect, right? It's good to know anyway.

masterT commented 8 years ago

Exactly!

masterT commented 8 years ago

@FdezRomero I just publish the new version of the package! :package:

FdezRomero commented 8 years ago

@masterT Thanks a lot! Hope to keep collaborating in the future.