Proposal: parseEndpoint without logging

[pwrnrd]

Personally I like to catch parseEndpoint myself without error logging. My implementation:

const parseEndpoint = (name, callOptions, opt) => async (parser) => {
    return parser(await callEndpoint(name, callOptions, opt));
};

The reason for this is that I use mws.getMarketplaces() to validate my API keys. If the call to getMarketplaces is unsuccesfull I get the warnings message **** caught parseEndpoint error. However, in the case where the API keys are invalid, the parseEndpoint error is not an error but the expected result. The console.warn call pollutes the console in that case.

[ericblade]

I'm glad to see someone using the brand new thing :-) I didn't expect that. It was something that I put in to try out a slightly different way of doing some things, especially since I knew I was going to need to re-use some of the parsers, digging into some of the newer stuff. (if/when I make changes to how that works, I'll make sure to add a separate call, instead of breaking how that works right off... i feel like i'm onto a good idea with parseEndpoint, but that I haven't fully realized it in my head just yet)

Definitely will remove the logging, and check to see how that works without the catch. I think the catch + re-throw was just so I could put in the log, and ensure that the error was getting caught in the right place . . .

[ericblade]

fyi @pwrnrd i'm messing with a "parseEndpoint2" that would probably replace the existing one. I had to use it with the current signature for a bit to see a better way :-)

const parseEndpoint2 = parser => name => async (callOptions, opt) => {

... which when used with something like getLowestPricedOffers* gives me

const getLowestPricedOffers2 = parseEndpoint2(parseLowestPricedOffers);
const getLowestPricedOffersForSKU2 = getLowestPricedOffers2('GetLowestPricedOffersForSKU');
const getLowestPricedOffersForASIN2 = getLowestPricedOffers2('GetLowestPricedOffersForASIN');

instead of the existing:

const getLowestPricedOffersForASIN = async (options) => {
    const results = await parseEndpoint('GetLowestPricedOffersForASIN', options)(parseLowestPricedOffers);
    return results;
};
const getLowestPricedOffersForSKU = async (options) => {
    const results = await parseEndpoint('GetLowestPricedOffersForSKU', options)(parseLowestPricedOffers);
    return results;
};

... and has the added bonus that it should allow to provide the callEndpoint options (ie adjusting throttling settings, saving data to files, etc) directly to the users of the wrappers . . which has been a bit of a pain point for me while developing some of the wrappers, and is now because i need to adjust some throttle options on my client side.

[ericblade]

ok, i've switched that up, and am using it now for all the calls that migrated. lmk if that works better? and i'll switch it to be the New Thing instead of the Experimental Thing. :-) (and rename it to 'parseEndpoint' and probably put it in it's own file)

[pwrnrd]

Hi Eric, thanks!! The code looks much cleaner this way. So thats already a great win.

Regarding the throttling options, just to see if I undertand you correctly, do you mean something like this?

const getLowestPricedOffers2 = parseEndpoint2(parseLowestPricedOffers);
const throttleOptions = {
    interval: 60000, //send request every minute
    limit: 60, // hourly request limit
    pauseAfterLimitReach: 3600000 // after reaching the limit wait an hour before sending requests again
}
getLowestPricedOffers2(offerData, throttleOptions)

[ericblade]

well, if you're going to call the new parseEndpoint (i'll drop the "2" next time i'm making a change)..

const parseLowestPricedOffers = (offers) => { ... }
const otherOptions = { ... }
const callOpt = { ASINList: [ '...'] } // i didn't look up the parameters to that call sorry :)
parseEndpoint(parseLowestPricedOffers)('GetLowestPricedOffersForASIN')(callOpt, otherOptions);

or closer to what you said:

const getLowestPricedOffers = parseEndpoint(parser)('GetLowestPricedOffersForASIN');
getLowestPricedOffers(callOpt, otherOptions);

right now, the otherOptions could be

 * @param {object} [opt] - options for callEndpoint
 * @param {boolean} [opt.noFlatten] - do not flatten results
 * @param {boolean} [opt.returnRaw] - return only the raw data (may or may not be flattened)
 * @param {string} [opt.saveRaw] - filename to save raw data to (may or may not be flattened)
 * @param {string} [opt.saveParsed] - filename to save final parsed data to (not compatible with returnRaw, since parsing won't happen)

and it looks like I forgot to document the one that affects throttling: maxThrottleRetries

I do like your ideas for interval, limit, pause . . . although i think having it understand the hourly request limit and having a really long retry period is pretty far beyond what it's able to do right now.

HOWEVER, now that mws-simple returns the hourly request information .. https://github.com/ericblade/mws-simple/commit/9121083bffed33628075dc3786d6ee123d171246

we can now receive all the headers and do something useful with them. So, we can know in advance when we're going to break quota, without having to tabulate all the data internally. The existing code works fine for the occasional 503, but it's not at all good for spacing out requests.

[pwrnrd]

Ah I see! Thanks!

HOWEVER, now that mws-simple returns the hourly request information .. ericblade/mws-simple@9121083 we can now receive all the headers and do something useful with them. So, we can know in advance when we're going to break quota, without having to tabulate all the data internally. The existing code works fine for the occasional 503, but it's not at all good for spacing out requests.

This will definitely help MWS developers finding problems with throttling in case that there are multiple parties performing requests on behalf of the same seller. I had to look this up but for developers the API limits are imposed on the Seller ID for which they perform the request: https://sellercentral.amazon.com/forums/t/do-mws-api-rate-limits-apply-per-seller-account-or-per-developer/122739 .

[ericblade]

parseEndpoint2 is about to become parseEndpoint, and it will reside in it's own file, if you're doing any work inside the lib at the moment.

[ericblade]

as of latest commit, there's lib/parseEndpoint if you're working inside the code, or you should be able to just call mws.parseEndpoint if you want to do it that way. (though callEndpoint probably makes more sense for most uses, if you're just 'await' on a call, parseEndpoint is more useful for internal code..)

i know i need to add documentation for all that new stuff i just put in, but it is...

(outParser, inParser = x => x) => apiName => async (apiOptions, callEndpointOptions)

so

await parseEndpoint(outputParser, inputParser)('APIFunctionName')({ ApiParameters }, { callParameters });

and you can macro-ize the first or second part of it