search

RobertSpread / spreadcart

simple plugin to display a cart for the spreadshop everywhere on your domain
1 stars 1 forks source link

Reading an invalid basket ID now returns an error #33

Open jtlapp opened 8 years ago

jtlapp commented 8 years ago

Up until recently, reading an invalid basket ID resulted in a successful HTTP request returning an error XML document. Sometime recently the API changed so that this request now returns an HTTP error and no XML document. This breaks the shopping cart, which was learning that a basket was no longer available by looking for the error XML rather than by looking for an error HTTP response. Looking into a fix now...

RobertSpread commented 8 years ago

Hi there, I was on vacation for the last weeks and had no access to any infrastructure. I will check what happened.

jtlapp commented 8 years ago

Looking into this further, it appears that the SpreadCart apiBasketId now changes as a result of making a purchase, and that may be the cause of the problem. I'll check to see if it's now being nulled-out. Nulling this value would be better behavior, because then I wouldn't have to make an API request to learn whether the cart has emptied. We just can't have it changing on us without notice.

jtlapp commented 8 years ago

Okay, I was wrong that the server has changed behavior, but apparently there are now circumstances where apiBasketId can become undefined. I'm not sure how this happens, but I'll add code to guard for it.

jtlapp commented 8 years ago

It's possible that this problem has been lurking all along and I only happened to encounter it. I should probably have guarded for this possibility anyway. The fix was to change this line:

    if(shopBasket !== null)

To this:

    if(shopBasket !== null && typeof shopBasket.apiBasketId !== "undefined"
            && shopBasket.apiBasketId !== null)

I think our versions are pretty out of synch, but I'll give it a look.

jtlapp commented 8 years ago

It looks like undoing that checkin from long ago has forever cursed us. It appears no longer possible to merge via github. I only have four simple changes to merge. github shows them to me on my proposed pull request and then says it must be done manually.

I'll go ahead and make the pull request, just in case you see an easy way to resolve this. Maybe your engineering team has a suggestion for undoing this forever problem? I've been locally merging every single one of my changes before checking in so that github will auto-merge them for you. I'll hold off on doing that this time around, in case you or your team have a solution for us. Thanks!

jtlapp commented 8 years ago

I'm inclined to go through the code doing some defensive programming. For example, instead of relying on spreadshirt to return an XML document if I ask for a basket that was already checked out, I should also look for a resource-not-found error and handle that equivalently.