GetFulfillmentOrder casts Seller Order Ids to Integers

[fny]

I have some order ids that are numbers (e.g. "23914352147") which fail when calling getFulfillmentOrder.

I think Amazon is responding with a number rather than a string which is causing the codec to choke. Still trying to figure out a patch, guidance is appreciated.

mwsClient.fulfillmentOutboundShipment.getFulfillmentOrder({ SellerFulfillmentOrderId: '23914352147' })

ParsingError: Problem with the value of property "GetFulfillmentOrderResponse": Problem with the value of property "GetFulfillmentOrderResult": Problem with the value of property "FulfillmentOrder": Problem with the value of property "SellerFulfillmentOrderId": Expected a string, but received a number with value 23914352147
    at Object.Left (C:\Users\Faraz\Workspace\amazon-reports\node_modules\@scaleleap\amazon-mws-api-sdk\lib\sections\fulfillment-outbound-shipment\fulfillment-outbound-shipment.js:110:23)
    at Left.caseOf (C:\Users\Faraz\Workspace\amazon-reports\node_modules\purify-ts\Either.js:273:58)
    at FulfillmentOutboundShipment.getFulfillmentOrder (C:\Users\Faraz\Workspace\amazon-reports\node_modules\@scaleleap\amazon-mws-api-sdk\lib\sections\fulfillment-outbound-shipment\fulfillment-outbound-shipment.js:107:69)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async handler (C:\Users\Faraz\Workspace\amazon-reports\.next\server\pages\api\debug.js:580:13)
    at async Object.apiResolver (C:\Users\Faraz\Workspace\amazon-reports\node_modules\next\dist\server\api-utils.js:101:9)
    at async DevServer.handleApiRequest (C:\Users\Faraz\Workspace\amazon-reports\node_modules\next\dist\server\next-server.js:760:9)
    at async Object.fn (C:\Users\Faraz\Workspace\amazon-reports\node_modules\next\dist\server\next-server.js:651:37)
    at async Router.execute (C:\Users\Faraz\Workspace\amazon-reports\node_modules\next\dist\server\router.js:205:32)
    at async DevServer.run (C:\Users\Faraz\Workspace\amazon-reports\node_modules\next\dist\server\next-server.js:825:29)

[moltar]

Can you please confirm which version of the package you are using?

[moltar]

Most likely need to add an entry here:

https://github.com/ScaleLeap/amazon-mws-api-sdk/blob/dd73a9bc9602a5a6fa0a96a2587c738eee5bba24/src/http.ts#L348

Please add a test case with some data.

Perhaps by editing one of the existing files for tests, and add a number-lile value:

https://github.com/ScaleLeap/amazon-mws-api-sdk/blob/58f39f25dfc43e2b222bf2bf0e7adc7d51f281ef/test/unit/__fixtures__/fulfillment_outbound_shipment_list_all_fulfillment_orders_nt.xml#L11

And then re-run the tests with -u to update the snapshots.

[fny]

It's the latest version 1.9.56

There was some attempt to fix this within ensureString, but it didn't behave the way I thought it would. I'll try to get this fixed tonight. Thanks for being so responsive!

https://github.com/ScaleLeap/amazon-mws-api-sdk/blob/dd73a9bc9602a5a6fa0a96a2587c738eee5bba24/src/parsing.ts#L37

[fny]

Quick q: do you know how to pin a module to a local version? I've tried the line below, and node keeps complaining it can't resolve @scaleleap/amazon-mws-api-sdk.

    "@scaleleap/amazon-mws-api-sdk": "file:../amazon-mws-api-sdk",

[moltar]

Quick q: do you know how to pin a module to a local version? I've tried the line below, and node keeps complaining it can't resolve @scaleleap/amazon-mws-api-sdk.

    "@scaleleap/amazon-mws-api-sdk": "file:../amazon-mws-api-sdk",

Maybe it's because the original is in TS and node cannot find the built files.

Have you tried running npm run build in the mws package repo first? This should put the built files into lib/.

But you'll need to re-run it every time you make a change.

[moltar]

Also, I found this useful before: https://github.com/wclr/yalc

[fny]

I've solved it. The trick is to use the ensureString codec rather than the string codec wherever SellerFulfillmentOrderId is defined. I don't think this is a good long term solution however.

The core issue is that XML can't discriminate between a number or a string, so if anything looks remotely like a number, it will be parsed as such. You guys already picked up on this issue with things like postal code which likely broke for leading zeros:

<PostalCode>02770</PostalCode>

Given the current codecs, this bug will keep popping up. Imagine a phone number provided without dashes or an apartment number. There's no reliable way to guarantee any string field might not look like a number.

I think a better solution would be to have the XML parser return strings by default (parseNodeValue: false) and then use a special codec to transform the values you know should be numbers to numbers (i.e. Qty) since theres an absolute guarantee Amazon isn't going to return something else.

Let me know what you think!

[moltar]

I think you are totally right. But this is probably a massive change though. However, if you feel like doing this, I will certainly accept and merge the PR.

I have just done a lot of clean-up on the CI and test infra. It should be easier to submit PRs as tests will run properly without failure.

[moltar]

A way to get to quick failure and iteration is probably flip the parseNodeValue flag and then see what fails during snapshot tests. Then go and fix that codec.

[moltar]

Fixed in #422