fix: object.patch.rmLink not working #1508

alanshaw · 2018-08-15T10:51:54Z

This PR contains fixes for the test failures that are happening in master for object.patch.rmLink.

I think this change ipld/js-ipld-dag-pb#80 made DAGLink validate the hash better and caused the failures to start.

It also contains fixes for the pinSet module which was using the "private" _multihash property which was removed in ipld/js-ipld-dag-pb#81 and released 2 days ago - don't use private APIs kids.

License: MIT Signed-off-by: Alan Shaw <[email protected]>

achingbrain

I think we might want to make ipfs.object.patch.rmLink take a string arg (e.g. link name) in future.

alanshaw · 2018-08-15T11:32:55Z

:( looks like there's now problems with pinSet

achingbrain · 2018-08-15T11:45:37Z

I'm sure you've found it already but it looks like passing '' as the multihash is the cause:

https://github.com/ipfs/js-ipfs/blob/master/src/core/components/pin-set.js#L115
https://github.com/ipfs/js-ipfs/blob/master/src/core/components/pin-set.js#L121

  pinSet storeItems
Debug: internal, implementation, error 
    AssertionError [ERR_ASSERTION]: A link requires a multihash to point to
    at new DAGLink (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/ipld-dag-pb/src/dag-link/index.js:10:5)
    at new DAGLink (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/class-is/index.js:15:17)
    at pins.map.item (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:121:21)
    at Array.map (<anonymous>)
    at storePins (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:120:14)
    at Object.storeItems (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:102:14)
    at Object.storeSet (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:91:14)
    at createNode (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/test/core/pin-set.js:77:16)
    at multihashing (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/ipld-dag-pb/src/dag-node/create.js:53:7)
    at Multihashing.Multihashing.digest (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/multihashing-async/src/index.js:33:5)
    at setImmediate (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/multihashing-async/src/utils.js:8:7)
    at Immediate.<anonymous> (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/setImmediate.js:27:16)
    at runCallback (timers.js:696:18)
    at tryOnImmediate (timers.js:667:5)
    at processImmediate (timers.js:649:5)
    at process.topLevelDomainCallback (domain.js:121:23)
      1) generates a root node with links and hash

  Suite duration: 5.002 s, Tests: 1

  pinSet handles large sets
Debug: internal, implementation, error 
    Error: Invalid version, must be a number equal to 1 or 0
    at Function.validateCID (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/cids/src/index.js:235:13)
    at new CID (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/cids/src/index.js:105:9)
    at toB58String (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:20:10)
    at hash (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:53:22)
    at pins.reduce (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:148:23)
    at Array.reduce (<anonymous>)
    at storePins (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:147:29)
    at Object.storeItems (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:102:14)
    at Object.storeSet (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:91:14)
    at createNodes (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/test/core/pin-set.js:99:16)
    at /tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/parallel.js:39:9
    at /tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/once.js:12:16
    at iterateeCallback (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/eachOfLimit.js:48:24)
    at /tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/onlyOnce.js:12:16
    at /tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/parallel.js:36:13
    at createNode (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/test/core/pin-set.js:34:44)
    at multihashing (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/ipld-dag-pb/src/dag-node/create.js:53:7)
    at Multihashing.Multihashing.digest (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/multihashing-async/src/index.js:33:5)
    at setImmediate (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/multihashing-async/src/utils.js:8:7)
    at Immediate.<anonymous> (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/setImmediate.js:27:16)
    at runCallback (timers.js:696:18)
    at tryOnImmediate (timers.js:667:5)
    at processImmediate (timers.js:649:5)
    at process.topLevelDomainCallback (domain.js:121:23)
      1) handles storing items > maxItems
      - stress test: stores items > (maxItems * defaultFanout) + 1

  Suite duration: 19.001 s, Tests: 2

  pinSet walkItems
      ✓ fails if node doesn't have a pin-set protobuf header: 1ms
Debug: internal, implementation, error 
    AssertionError [ERR_ASSERTION]: A link requires a multihash to point to
    at new DAGLink (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/ipld-dag-pb/src/dag-link/index.js:10:5)
    at new DAGLink (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/class-is/index.js:15:17)
    at pins.map.item (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:121:21)
    at Array.map (<anonymous>)
    at storePins (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:120:14)
    at Object.storeItems (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:102:14)
    at Object.storeSet (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/src/core/components/pin-set.js:91:14)
    at createNodes (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/test/core/pin-set.js:175:16)
    at /tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/parallel.js:39:9
    at /tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/once.js:12:16
    at iterateeCallback (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/eachOfLimit.js:48:24)
    at /tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/onlyOnce.js:12:16
    at /tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/parallel.js:36:13
    at createNode (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/test/core/pin-set.js:34:44)
    at multihashing (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/ipld-dag-pb/src/dag-node/create.js:53:7)
    at Multihashing.Multihashing.digest (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/multihashing-async/src/index.js:33:5)
    at setImmediate (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/multihashing-async/src/utils.js:8:7)
    at Immediate.<anonymous> (/tmp/jenkins/workspace/_js-ipfs_fix_object-rm-link-VLV2TBPORAS7C63UJSPGGK6BLAQUD4RI3ONWVTNAAKSYT5XGXBRQ/node_modules/async/internal/setImmediate.js:27:16)
    at runCallback (timers.js:696:18)
    at tryOnImmediate (timers.js:667:5)
    at processImmediate (timers.js:649:5)
    at process.topLevelDomainCallback (domain.js:121:23)
      1) visits all non-fanout links of a root node

...which was removed in ipld/js-ipld-dag-pb#81 License: MIT Signed-off-by: Alan Shaw <[email protected]>

daviddias · 2018-08-15T13:35:18Z

I'm sure you've found it already but it looks like passing '' as the multihash is the cause:

Please add a test for this and give the user a good error if it happens.

alanshaw · 2018-08-15T13:37:54Z

I'm sure you've found it already but it looks like passing '' as the multihash is the cause:

Please add a test for this and give the user a good error if it happens.

It was actually because the pinSet module was using the "private" _multihash property which was removed in ipld/js-ipld-dag-pb#81 and released 2 days ago. I've just switched this to use multihash and we're all good.

alanshaw · 2018-08-15T15:19:54Z

CI green - am merge ✨

fix: object.patch.rmLink not working

75c1a7b

License: MIT Signed-off-by: Alan Shaw <[email protected]>

ghost assigned alanshaw Aug 15, 2018

ghost added the status/in-progress In progress label Aug 15, 2018

alanshaw mentioned this pull request Aug 15, 2018

feat: adds data-encoding argument to control data encoding #1420

Merged

alanshaw requested review from achingbrain and daviddias August 15, 2018 10:58

achingbrain approved these changes Aug 15, 2018

View reviewed changes

fix: pinSet DAG link was using private _multihash prop

ff102e6

...which was removed in ipld/js-ipld-dag-pb#81 License: MIT Signed-off-by: Alan Shaw <[email protected]>

alanshaw merged commit afd3255 into master Aug 15, 2018

alanshaw deleted the fix/object-rm-link branch August 15, 2018 15:20

ghost removed the status/in-progress In progress label Aug 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: object.patch.rmLink not working #1508

fix: object.patch.rmLink not working #1508

alanshaw commented Aug 15, 2018 •

edited

Loading

achingbrain left a comment

alanshaw commented Aug 15, 2018

achingbrain commented Aug 15, 2018 •

edited

Loading

daviddias commented Aug 15, 2018

alanshaw commented Aug 15, 2018

alanshaw commented Aug 15, 2018

fix: object.patch.rmLink not working #1508

fix: object.patch.rmLink not working #1508

Conversation

alanshaw commented Aug 15, 2018 • edited Loading

achingbrain left a comment

Choose a reason for hiding this comment

alanshaw commented Aug 15, 2018

achingbrain commented Aug 15, 2018 • edited Loading

daviddias commented Aug 15, 2018

alanshaw commented Aug 15, 2018

alanshaw commented Aug 15, 2018

alanshaw commented Aug 15, 2018 •

edited

Loading

achingbrain commented Aug 15, 2018 •

edited

Loading