commercetools / sphere-node-sdk

Officially supported Node.js SDK library for working with the SPHERE.IO HTTP API, with OAuth2 support.
http://sphereio.github.io/sphere-node-sdk/
MIT License
16 stars 8 forks source link

Product-sync TypeError on attribute null values #215

Open butenkor opened 7 years ago

butenkor commented 7 years ago

Expected behavior

Should not throw an error if product attribute value for set of enums is null:

{
  "name": "laender",
  "value": [
    null
  ]
}

Actual behavior

{"name":"xxx","widget_type":"product-import","hostname":"xxx","pid":1,"level":50,"myMsg":"Error - Error on product sync. Skip and continue.","error":{"type":"TypeError","message":"Cannot set property '_MATCH_CRITERIA' of undefined","name":"TypeError","stack":"TypeError: Cannot set property '_MATCH_CRITERIA' of undefined\n at /app/nodemodules/sphere-node-sdk/lib/sync/utils/product.js:79:42\n at Function..each._.forEach (/app/node_modules/underscore/underscore.js:153:9)\n at /app/nodemodules/sphere-node-sdk/lib/sync/utils/product.js:77:22\n at Function..each._.forEach (/app/node_modules/underscore/underscore.js:153:9)\n at patchSetLText (/app/node_modules/sphere-node-sdk/lib/sync/utils/product.js:75:18)\n at /app/nodemodules/sphere-node-sdk/lib/sync/utils/product.js:103:9\n at Function..each._.forEach (/app/node_modules/underscore/underscore.js:153:9)\n at patch (/app/node_modules/sphere-node-sdk/lib/sync/utils/product.js:97:16)\n at ProductUtils.diff (/app/node_modules/sphere-node-sdk/lib/sync/utils/product.js:112:5)\n at ProductSync.BaseSync.buildActions (/app/node_modules/sphere-node-sdk/lib/sync/base-sync.js:26:24)\n at ProductSync.buildActions (/app/node_modules/sphere-node-sdk/lib/sync/product-sync.js:25:47)\n at /app/node_modules/sphere-product-import/dist/product-import.js:352:37\n at tryCatcher (/app/node_modules/sphere-product-import/node_modules/bluebird/js/main/util.js:26:23)\n at Promise._settlePromiseFromHandler (/app/node_modules/sphere-product-import/node_modules/bluebird/js/main/promise.js:507:31)\n at Promise._settlePromiseAt (/app/node_modules/sphere-product-import/node_modules/bluebird/js/main/promise.js:581:18)\n at Promise._settlePromises (/app/node_modules/sphere-product-import/node_modules/bluebird/js/main/promise.js:697:14)\n at Async._drainQueue (/app/node_modules/sphere-product-import/node_modules/bluebird/js/main/async.js:123:16)\n at Async._drainQueues (/app/node_modules/sphere-product-import/node_modules/bluebird/js/main/async.js:133:10)\n at Immediate.Async.drainQueues (/app/node_modules/sphere-product-import/node_modules/bluebird/js/main/async.js:15:14)\n at runCallback (timers.js:649:20)\n at tryOnImmediate (timers.js:622:5)\n at processImmediate [as _immediateCallback] (timers.js:594:5)"},"msg":"","time":"2017-02-19T06:40:35.105Z","v":0}

Steps to reproduce the behavior

hisabimbola commented 7 years ago

Hi @butenkor

According to the documentation here, the attribute value is set to null when you want to remove it from the variant.

So instead of passing null in an array, passing is as the value does not throw an error e.g

{
  "name": "laender",
  "value": null
}

Or can you provide a valid use case where there is a need to have null in an array.

butenkor commented 7 years ago

We noticed this issue as there was an error in JSON structure of data to import so values were not properly populated. If we want to delete an attribute or unset it than we just do not provide it in new_object so that sync can calculate attribute deletion automatically (it works already). Here it is obvious that value of enum is wrong but this value should go to API and API should response with 400 error. Instead whole import process is endangered as sync module throws TypeError.

daern91 commented 6 years ago

Hi @butenkor, does this still have to be fixed since we are deprecating?

butenkor commented 6 years ago

It does not have high priority for us as we applied workaround on our side but i see at as a bug as one can continue on 400 API errors but can not on JS TypeError as you never know what is behind it so the whole process gets aborted and requires immediate fix release and rollout (otherwise process would get stuck on same error repeatedly). It would be nice if it could be fixed.