Creating sponsored products reports fails

[SaturnFromTitan]

Hi! First off: Thanks for this SDK, I very much like it so far!

Describe the bug
Requesting a sponsored products campaigns report fails for me. When building the request myself with requests I can get it to work, so it's definitely not an issue with the API or with my credentials.

To Reproduce
When I run the following script

credentials = {'refresh_token': refresh_token, 'client_id': client_id, 'client_secret': client_secret, 'profile_id': profile_id}
client = Reports(marketplace=Marketplaces.EU, credentials=credentials)

body = {
    "reportDate": "20220101",
    "metrics": "campaignId,impressions,clicks,cost",
}
res = client.post_report(recordType="campaigns", body=body)
print(res)

I receive the following error:

{'headers': ...,
 'next_token': None,
 'payload': {'code': '422',
             'details': "Unrecognized token 'reportDate': was expecting",
             'requestId': '1H1P1JK3PJ70DD33WRY3'}}

Expected behaviour
Receive a 202 response

Solution
I managed to find the source of the bug when building my own request with requests:

Instead of using request.post's data parameter, you need to pass body to json.

I.e. this line: https://github.com/denisneuf/python-amazon-ad-api/blob/main/ad_api/api/sp/reports.py#L33

needs to be changed to
return self._request(fill_query_params(kwargs.pop('path'), recordType), json=kwargs.pop('body'), params=kwargs)

Then everything should be working fine.

I'm happy to provide a PR to fix this. I wait for a reply here first though ✌️

[denisneuf]

Hi Saturn,
first of all sorry for the late response but in China we had a few days holidays for the Chinese New Year :)
About your issue I read your example and you are right as the body need to be passed as json, but when I was preparing the endpoints I though the method manipulate as less as possible the data submitted by the client then you could have more control on the way you obtain the data, for example if the data is stored in a .json or .xml file and you need to read it you can do your own script just composing a valid json as body parameter.

So in my personal examples to test the same endpoint in the sponsored_brands I have something similar like you:

    record_type = "keywords"
    report_segment = "query"
    report_date = "20211109"
    report_metrics = "impressions,clicks"

    dictionary = {
        "segment": report_segment,
        "reportDate": report_date,
        "metrics": report_metrics
    }

    result = sponsored_brands.Reports(account=store, marketplace=marketplace, debug=True).post_report(
        recordType=record_type,
        body=json.dumps(dictionary)
    )

    payload = result.payload

    logging.info(payload)

So according to your code it could be easily solved without modifying the endpoint like this:

credentials = {'refresh_token': refresh_token, 'client_id': client_id, 'client_secret': client_secret, 'profile_id': profile_id}
client = Reports(marketplace=Marketplaces.EU, credentials=credentials)

body = {
    "reportDate": "20220101",
    "metrics": "campaignId,impressions,clicks,cost",
}
res = client.post_report(recordType="campaigns", body=json.dumps(body))
print(res)

How do you think?

Kindest regards.

[SaturnFromTitan]

Ah, good point - that's a good solution indeed! Especially, because it avoids breaking the API of these methods.

I still wonder if the method could be more flexible in that it checks the type of body and then either uses the data or json parameter accordingly 🤔 If you don't like that, maybe at least a warning could be emitted?

Initially, I was just very confused by the error message saying "Unrecognized token 'reportDate': was expecting". I didn't get that it was about the data type and started studying the API docs again to see if my body dictionary was malformed instead 🙈

[denisneuf]

I still wonder if the method could be more flexible in that it checks the type of body and then either uses the data or json parameter accordingly 🤔 If you don't like that, maybe at least a warning could be emitted?

Could you write a pseudo code that shows what would you like to add? we can add it as a pull request if you think it is necessary and there is no back compatibility problem.

[SaturnFromTitan]

Sure. I mean something like this:

    def post_report(self, recordType, **kwargs) -> ApiResponse:
        body = kwargs.pop('body')
        if not isinstance(body, (str, bytes):
            try:
                body = json.dumps(body)
            except TypeError:
                pass
        return self._request(fill_query_params(kwargs.pop('path'), recordType), data=kwargs.pop('body'), params=kwargs)

Instead of implicitly casting body to a json string, you could simply emit a warning:

    def post_report(self, recordType, **kwargs) -> ApiResponse:
        body = kwargs.pop('body')
        if not isinstance(body, (str, bytes):
            warnings.warn("`body` will likely need to be a string. You might want to use `json.dumps`.", UserWarning)
        return self._request(fill_query_params(kwargs.pop('path'), recordType), data=kwargs.pop('body'), params=kwargs)

Wdyt?

[denisneuf]

I prefer just a warning style so you could prepare a PR and I will test it.