Shopify / js-buy-sdk

The JS Buy SDK is a lightweight library that allows you to build ecommerce into any website. It is based on Shopify's API and provides the ability to retrieve products and collections from your shop, add products to a cart, and checkout.
https://shopify.github.io/js-buy-sdk
MIT License
984 stars 259 forks source link

Issue with selectedVariant checkoutUrl #318

Closed lucasmoeskops closed 7 years ago

lucasmoeskops commented 7 years ago

In the latest version we are having an issue with generating the checkout url of a product selectedVariant. The accessToken variable in the url is empty. It seems that in the product.config only 'accessToken' is known, whilst the checkoutUrl function tries to get 'apiKey' from the config.

dvdjrnx commented 7 years ago

I seem to be running into this problem as well. It's not an ideal solution, but this is what I've done to get a working URL for now:

var initURL = product.selectedVariant.checkoutUrl(1);
var subURL = initURL.substring(0, initURL.lastIndexOf('?') + 1);
var checkoutURL = subURL + 'access_token=' + token + '&_fd=0';

token in the checkoutURL string, in case it isn't clear, is a variable that I've set to my access token.

Thank you for posting about this @lucasmoeskops! I had been struggling with it for a while and you pointed me in the right direction for a (temporary) solution :)

swalkinshaw commented 7 years ago

v0.7.1 was published with this fix

alexandervlpl commented 7 years ago

Because of this bug, my client's shop has been quietly broken for weeks. This glaring issue breaks the main (and practically the only) use case for this SDK.

Is Shopify actually involved in maintaining this SDK? Some questions for them:

  1. Do they even test before making releases?
  2. It looks like their CDN is still serving the broken version. Why? If they're telling developers to use a CDN, fixes like this need to be rolled out ASAP. Also: I would really recommend providing fixed, tested releases on the CDN instead of whatever "latest" means. It's common practice and it prevents situations like this.
  3. Did they notify their paying customers about this issue, so they could act accordingly and keep their stores running? Doesn't look like it.

In short, this is exactly the kind of clusterf**k that a professional team would a) avoid and b) handle quickly and transparently when it does happen. Looks like Shopify is capable of neither.

@swalkinshaw : thanks for the fix! @dvdjrnx : thanks for the workaround!

swalkinshaw commented 7 years ago

@alexandervlpl our apologies for not updating the CDN version. This was a result of a not ideal, 2-step deployment process which can lead to inconsistencies like this. We are in the process of fixing this so it doesn't happen again. See https://github.com/Shopify/js-buy-sdk/pull/325.

In the meantime, the CDN version has now been updated to reflect the latest version (0.7.1). We do provide specific versioned files on the CDN although we're missing documentation around that. I'll add a note about how to access them.

And thanks for letting us know about this issue. Obviously it's frustrating to run into a bug like this but we'd appreciate comments more in line with our Contributor Code of Conduct in the future.

alexandervlpl commented 7 years ago

@swalkinshaw: thanks for fixing the CDN issue so quickly and for acknowledging the problem. Can you please provide a CDN URL for the current release? I tried a few obvious ones, but no luck.

Maybe you can also improve your testing and release process? It's really core functionality that was broken here, and there are ways to catch things like this.

I do respect my fellow developer and I know that mistakes happen. You have to admit though... there's no nicer way to describe an issue that probably broke thousands of shops around the world for weeks, and your subsequent failure to notify users about it.

I really hope the communication failure was an oversight and not a deliberate decision by your team.

Users like my client spent time and money promoting their shops during these weeks, and if anything they only damaged their online presence because customers could not order. I'm amazed that this can happen with what is probably the largest e-commerce platform on the web. If I were Shopify, I'd be looking for ways to make it up to these users right now.

swalkinshaw commented 7 years ago

@alexandervlpl here's all the files we publish to our CDN for a release:

http://sdks.shopifycdn.com/js-buy-sdk/v0/latest/shopify-buy.umd.js http://sdks.shopifycdn.com/js-buy-sdk/v0/latest/shopify-buy.umd.min.js http://sdks.shopifycdn.com/js-buy-sdk/0.7.1/shopify-buy.umd.js http://sdks.shopifycdn.com/js-buy-sdk/v0/latest/shopify-buy.umd.polyfilled.js http://sdks.shopifycdn.com/js-buy-sdk/0.7.1/shopify-buy.umd.polyfilled.min.js http://sdks.shopifycdn.com/js-buy-sdk/0.7.1/shopify-buy.umd.min.js http://sdks.shopifycdn.com/js-buy-sdk/0.7.1/shopify-buy.umd.polyfilled.js http://sdks.shopifycdn.com/js-buy-sdk/v0/latest/shopify-buy.umd.polyfilled.min.js

In terms of communication, we didn't know about the issue until your post today. Otherwise it would have been fixed ASAP.

Can you clarify what has been broken your client's shop? From our understanding this would affect checkout attribution but checkout itself should have still worked. Is that correct? Just trying to determine the scope of this.

alexandervlpl commented 7 years ago

@swalkinshaw Thanks for the URLs. Is it safe to use 0.7.1 for the foreseeable future, or is there a chance that it will no longer be supported?

I reproduced what was happening in the shop by switching back to the broken release (0.7.0): http://sdks.shopifycdn.com/js-buy-sdk/0.7.0/shopify-buy.umd.polyfilled.min.js

The shop front end was doing this (basic checkout process for one item, always worked):

var url = product.selectedVariant.checkoutUrl(1);
window.open(url, '_blank');

With 0.7.0, this opened a URL like this one. This is an actual URL I just got from the SDK (for my test shop): https://mapart-test-store.myshopify.com/cart/34320196812:1?access_token=&_fd=0

You should be able to open it and see a Shopify error page. Checkout itself was broken, not just attribution: fail

So depending on how long 0.7.0 was actually on your "latest" CDN, this probably broke my client's shop for ~20 days? I actually thought you saw the full extent of the problem 14 days ago, when this issue was reported by @lucasmoeskops . Hence my shock that nothing was done sooner or communicated to users, and my use of the word clusterf**k. :wink:

swalkinshaw commented 7 years ago

@alexandervlpl specific versions are safe to use unless we notify otherwise and we'd give notice. People can and do use specific versions via NPM already.

We do have a new major rewrite and version underway. The "latest" naming scheme is meant to mirror semantic versioning so it's tied to minor versions of 0.x. If we follow semantic versioning properly then you can safely use any new minor version (ie: 0.6.0 -> 0.7.0 but not 1.0.0). Of course this doesn't help when an actual bug/regression happens like this one.

Our new major version wouldn't get automatically pushed as the v0 "latest" since it will have breaking changes. We'd have a v1 "latest" but you'd have to explicitly update your site to use it.

Thanks for trying out that older version and letting us know what was broken. In terms of the timeline, you're correct about the ~20 days (23 I believe). What we didn't realize was 1) the severity of it, 2) that it wasn't actually fixed on the CDN for the past ~8 days.

A lot of little things actually contributed to cause this avoidable bug as often happens. Integration tests to catch an issue like this is the hardest kind since it's relying on an external site, but I'm going to look into how we can improve them so something like this doesn't happen again.