Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amazon Apis sends campaignId as number/integer #975

Open
NishealJ opened this issue Aug 4, 2022 · 9 comments · May be fixed by #976
Open

Amazon Apis sends campaignId as number/integer #975

NishealJ opened this issue Aug 4, 2022 · 9 comments · May be fixed by #976

Comments

@NishealJ
Copy link
Contributor

NishealJ commented Aug 4, 2022

I came through a problem where I can see clear disparency in data.

When I send the request from postman the data is different and when I send it from amazon-advertising-api-sdk the data is different.

The reason is amazon ads API send campaignId as a number https://advertising.amazon.com/API/docs/en-us/sponsored-products/2-0/openapi#/Campaigns/listCampaigns and javascript cannot support long numbers so it just rounds offs.

For example, when I hit the API through postman the campaign id is 123456789123456789 but when I hit from sdk it becomes 123456789123456780.

Any work around this? @moltar @nguyentoanit

Screenshot 2022-08-04 at 8 04 21 PM

Screenshot 2022-08-04 at 7 59 27 PM

@moltar
Copy link
Contributor

moltar commented Aug 4, 2022

Hey NishealJ, thank you for opening this issue.

We are aware of this problem and have been in a discussion with Amazon Ads API support. We did not arrive at any solutions yet.

The value is documented as a number type, and yes, as you have correctly pointed it out, it is a "bigint", and thereby JS cannot handle it.

But the value is numerical in JSON that comes from the API. So I don't understand how a JSON parser in JS is supposed to handle it?

{
 "campaignId": 123456789123456789
}

I think it is actually wrong to use the ID as a number, and should have been a string, but I didn't design the API 😁

Do you have any suggestions?

@NishealJ
Copy link
Contributor Author

NishealJ commented Aug 4, 2022

Correct @moltar, now bigInt campaignIds are more frequent.
So javascript HTTP client will actually try to parse the response and because of bigInt, it won't parse it the right way.

Right now, I also don't see any solution but I would try to get a plain text response and then play with it.
Sending campaignId as a 64-bit integer clearly makes ads API unsupported in JS right?

@moltar
Copy link
Contributor

moltar commented Aug 5, 2022

Sending campaignId as a 64-bit integer clearly makes ads API unsupported in JS right?

Yeah, pretty much.

Right now, I also don't see any solution but I would try to get a plain text response and then play with it.

That is what I am thinking.

But there are other caveats though. I have tried using bigint stuff, but then it will stringify internally and can lead to other type issues later in the code.

If JS will coerce it to string, and you send it back down the API pipe, the API may not accept the string version of the ID.

Maybe we could use: https://www.npmjs.com/package/json-bigint

@moltar
Copy link
Contributor

moltar commented Aug 5, 2022

The weirdest part is that Amazon's own UI uses these IDs too, and it is also using browser JSON.parse 🤷🏼 I really don't get it!

@moltar moltar linked a pull request Aug 5, 2022 that will close this issue
@moltar
Copy link
Contributor

moltar commented Aug 5, 2022

@NishealJ Started a PR attempt, but ran into too many errors in the parser and have to shelve it for now. If you feel like picking it up, please do!

@AdsAPIPM
Copy link

AdsAPIPM commented Aug 5, 2022

We are currently working on the next version of this API where this ID will be modeled as a string. Would that solve this problem?

@moltar
Copy link
Contributor

moltar commented Aug 5, 2022

@AdsAPIPM It certainly would. Thanks for jumping in with the discussion!

Generally, I believe all IDs should be strings.

My sort-a heuristic is like this: will we do calculations with it? If no, then it is a string.

In other words, does it make sense to do ID1 + ID2? In 99% of cases the answer is no. So even if as a developer I decide to use ints or bigints, I'd stringify it, and document it as a "dumb string".

@NishealJ
Copy link
Contributor Author

NishealJ commented Aug 7, 2022

fix: convert number IDs to BigInt #976

Cool, I'll try to pull & help too.

@NishealJ
Copy link
Contributor Author

NishealJ commented Aug 7, 2022

@AdsAPIPM yes that would help a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants