Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Uncaught Error: Can't set property: 'links' is immutable #1131

Closed
haadcode opened this issue Dec 6, 2017 · 37 comments · Fixed by ipfs/aegir#180, ipld/js-ipld-dag-pb#58 or multiformats/js-multiaddr#55
Assignees
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@haadcode
Copy link
Member

haadcode commented Dec 6, 2017

After updating to 0.27.0 and running in the browser, there's an error that didn't happen before:

Uncaught Error: Can't set property: 'links' is immutable
    at module.exports.set links [as links] (index.min.js:1)
    at Object.Buffer.serialize (index.min.js:1)
    at cb (index.min.js:1)
    at nextTask (index.min.js:1)
    at exports.default (index.min.js:1)
    at IPLDResolver._put (index.min.js:1)
    at IPLDResolver.put (index.min.js:1)
    at DAGNode.create (index.min.js:1)
    at multihashing (index.min.js:1)
    at Multihashing.Multihashing.digest (index.min.js:1)

Version: 0.27.0
Platform: Browser

This happens upon starting IPFS, but it never starts as the error is thrown. Doesn't happen with 0.26.0.

Full stack trace for what it's worth (index.min.js is ipfs/dist/index.min.js):

set links @ index.min.js:1
Buffer.serialize @ index.min.js:1
cb @ index.min.js:1
nextTask @ index.min.js:1
exports.default @ index.min.js:1
_put @ index.min.js:1
put @ index.min.js:1
DAGNode.create @ index.min.js:1
multihashing @ index.min.js:1
Multihashing.Multihashing.digest @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ orbitdb.min.js:1
i @ orbitdb.min.js:1
n @ orbitdb.min.js:1
postMessage (async)
o @ orbitdb.min.js:1
u.setImmediate @ orbitdb.min.js:1
(anonymous) @ index.min.js:1
Promise resolved (async)
nodeify @ index.min.js:1
(anonymous) @ index.min.js:1
sha2256 @ index.min.js:1
Multihashing.digest @ index.min.js:1
Multihashing @ index.min.js:1
serialize @ index.min.js:1
Buffer.serialize @ index.min.js:1
module.exports @ index.min.js:1
new.promisify @ index.min.js:1
(anonymous) @ index.min.js:1
cb @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ index.min.js:1
exports.default @ index.min.js:1
exports.default @ index.min.js:1
exports.default @ index.min.js:1
cb @ index.min.js:1
nextTask @ index.min.js:1
next @ index.min.js:1
(anonymous) @ index.min.js:1
closed.err @ index.min.js:1
(anonymous) @ index.min.js:1
next @ index.min.js:1
(anonymous) @ index.min.js:1
closed.cb @ index.min.js:1
nextTask @ index.min.js:1
next @ index.min.js:1
(anonymous) @ index.min.js:1
closed.waterfall @ index.min.js:1
nextTask @ index.min.js:1
next @ index.min.js:1
(anonymous) @ index.min.js:1
callback @ index.min.js:1
setImmediate @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ orbitdb.min.js:1
i @ orbitdb.min.js:1
n @ orbitdb.min.js:1
postMessage (async)
o @ orbitdb.min.js:1
u.setImmediate @ orbitdb.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ index.min.js:1
module.exports @ index.min.js:1
closed.cb @ index.min.js:1
nextTask @ index.min.js:1
next @ index.min.js:1
(anonymous) @ index.min.js:1
closed.waterfall @ index.min.js:1
nextTask @ index.min.js:1
next @ index.min.js:1
(anonymous) @ index.min.js:1
setImmediate @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ orbitdb.min.js:1
i @ orbitdb.min.js:1
n @ orbitdb.min.js:1
postMessage (async)
o @ orbitdb.min.js:1
u.setImmediate @ orbitdb.min.js:1
(anonymous) @ index.min.js:1
exports.lock @ index.min.js:1
closed.cb @ index.min.js:1
nextTask @ index.min.js:1
next @ index.min.js:1
(anonymous) @ index.min.js:1
parallel @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ index.min.js:1
iterateeCallback @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ index.min.js:1
get @ index.min.js:1
store.get @ index.min.js:1
(anonymous) @ index.min.js:1
req.onsuccess @ index.min.js:1
IndexedDB (async)
Level._get @ index.min.js:1
AbstractLevelDOWN.get @ index.min.js:1
LevelUP.get @ index.min.js:1
get @ index.min.js:1
get @ index.min.js:1
check @ index.min.js:1
version @ index.min.js:1
(anonymous) @ index.min.js:1
replenish @ index.min.js:1
(anonymous) @ index.min.js:1
exports.default @ index.min.js:1
(anonymous) @ index.min.js:1
exports.default @ index.min.js:1
exports.default @ index.min.js:1
exports.default @ index.min.js:1
_isInitialized @ index.min.js:1
closed.cb @ index.min.js:1
nextTask @ index.min.js:1
next @ index.min.js:1
(anonymous) @ index.min.js:1
err @ index.min.js:1
(anonymous) @ index.min.js:1
Item.run @ index.min.js:1
drainQueue @ index.min.js:1
setTimeout (async)
runTimeout @ index.min.js:1
process.nextTick @ index.min.js:1
LevelUP.open @ index.min.js:1
open @ index.min.js:1
closed.cb @ index.min.js:1
nextTask @ index.min.js:1
exports.default @ index.min.js:1
open @ index.min.js:1
waterfall @ index.min.js:1
nextTask @ index.min.js:1
next @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ index.min.js:1
replenish @ index.min.js:1
iterateeCallback @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ index.min.js:1
(anonymous) @ index.min.js:1
tx.oncomplete @ index.min.js:1
@haadcode haadcode added the kind/bug A bug in existing code (including security flaws) label Dec 6, 2017
@daviddias
Copy link
Member

@haadcode that property has been immutable for a while https://github.com/ipld/js-ipld-dag-pb/blob/master/src/dag-node/index.js#L51-L53 wonder if you changed something in your codebase that tries to change the links property.

You can still use the class functions as you could before, such as .addLink, see docs at https://github.com/ipld/js-ipld-dag-pb#table-of-contents

@pgte
Copy link
Contributor

pgte commented Dec 7, 2017 via email

@daviddias
Copy link
Member

@pgte @haadcode can you create a repro case to test it out?

@haadcode
Copy link
Member Author

haadcode commented Dec 9, 2017

This only happens with a minified build of ipfs (dist/index.min.js). Will try to provide a simple test case, but this sounds like uglify (or webpack) issue.

@haadcode
Copy link
Member Author

haadcode commented Dec 9, 2017

If I pull js-ipfs from master, build it and npm link it to the project this is happening, the error is gone. Will investigate more.

git clone https://github.com/ipfs/js-ipfs.git
cd js-ipfs/
npm install && npm run build

Produces dist/ipfs.min.js that works and the error doesn't come up.

npm install ipfs

And using dist/ipfs.min.js from the npm package produces the aforementioned error.

@diasdavid perhaps something went funky in the release and the up-to-date dist file was not included?

@daviddias
Copy link
Member

Seems that #1136 also had issues. Let me try to publish again.

@daviddias
Copy link
Member

@haadcode could you try with latest?

@haadcode
Copy link
Member Author

@diasdavid I can confirm that it still happens with 0.27.3 :/ Using dist/index.min.js produces the aforementioned error while dist/index.js works as expected.

However, I have good news, I think :) I was able to get everything working correctly with webpack, compiling and uglifying orbitdb and ipfs together, by setting the mangle and compress flags to false:

const uglifyOptions = {
  uglifyOptions: {
    mangle: false,
    compress: false,
  },
}

When uglified non-compressed, it seems to work as expected while still minifying the output bundle. So what I take from this is that in order to fix dist/index.min.js, it should be uglified without compressing it? Or find out why compressing it is now causing it to throw an error. Perhaps changes in Uglify?

@louisgv
Copy link

louisgv commented Jan 7, 2018

I'm using 0.27.4 from jsdelivr cdn and this error was thrown. Sample code:

  const obj = {
            Data: Buffer.from('Some data'),
            Links: []
          }

          ipfs.object.put(obj)
  

@ya7ya
Copy link
Contributor

ya7ya commented Feb 17, 2018

hey, this was fixed in a quick way , which is not to mangle or compress when uglifying. this however means if js-ipfs is used in another project, this project cannot mangle or compress the code.

vmx pushed a commit to ipld/js-ipld-dag-pb that referenced this issue Mar 8, 2018
This is part of ongoing effort to properly fix ipfs/js-ipfs#1131.
vmx pushed a commit to multiformats/js-multiaddr that referenced this issue Mar 8, 2018
Replace constructor.name with instanceof. This is part of ongoing effort to properly
fix ipfs/js-ipfs#1131.
@vmx
Copy link
Member

vmx commented Mar 9, 2018

Thanks @ya7ya for your pull requests. The problem is that using instanceof might solve this issue, but it will lead to others (you can't use a differently imported module even if it's the same code). So we want to stick with constructor.name unless there's a better solution.

Do I understand correctly that this issue happens when you bundle js-ipfs with your own project an minify it? So it doesn't happen if you use the standalone minified version? If that's the case then we should document in the README which options you need to set on uglifyjs if you want to minify it together with your project.

Credit where credit is due: Thanks @dryajov and @dignifiedquire for looking into this.

@daviddias
Copy link
Member

@vmx note the bad merge here multiformats/js-multiaddr#55

@jellegerbrandy
Copy link

jellegerbrandy commented Mar 14, 2018

A workaround that would allow for minifying while not breaking anything in the non-minified versions would be two do both checks, and replace the pattern

  (thing.constructor.name === 'SomeThing')

with

 (thing instanceOf Something || thing.constructor.name === 'Something')

It is not very pretty, and still would mean that there would remain to be a difference in behavior after minitifying (but less than before), but for us, but at least it would allow for minifying, and it would not break anything existing afaics.

@diasdavid @ya7ya

@vmx
Copy link
Member

vmx commented Mar 14, 2018

Wouldn't the safest workaround be to gather a list of classes that do such a check and excluding those from the minifaction via an UglifyJS setting?

@mitra42
Copy link

mitra42 commented Mar 15, 2018

@vmx - wont you run into other problems with constructor.name, i.e. that anyone subclassing is going to fail your test? @jellegerbrandy's solution looks like a good one, partly since someone including js-ipfs into their project might not know much about the IPFS constraint on not minifying at all, especially if they include something that includes IPFS.

(For example our archive.org demo page requires our Dweb library which requires IPFS, currently I'm maintaining all three, but that's unlikely to be the case once the project scales), and coincidentally it was starting to concern me that things that work in my development version (not minified) failed in IPFS in the 'production' (i.e. minified) version.

@pgte
Copy link
Contributor

pgte commented Mar 15, 2018

@vmx @mitra42 @jellegerbrandy What about creating an is* function by using symbols as described here?

Something like this:

const typeSymbol = Symbol.for('whatever-type-symbol');

class Whatever {
  static isWhatever(obj) {
    return obj && Boolean(obj[typeSymbol]);
  }
  constructor() {
    this[typeSymbol] = true;
  }
}

const whatever = new Whatever();
Whatever.isWhatever(whatever); // true

@fsdiogo
Copy link
Contributor

fsdiogo commented Apr 23, 2018

Check #1321 for more info.

@mitra42
Copy link

mitra42 commented Jun 7, 2018

Is uglify working in the current (0.29.3) js-ipfs yet. Its hard to tell because the breakages are subtle.

@fsdiogo
Copy link
Contributor

fsdiogo commented Jun 7, 2018

It is @mitra42. But if you have a custom webpack config, you have to pass this option to UglifyJS:

compress: {
	unused: false
}

@mitra42
Copy link

mitra42 commented Jun 11, 2018

@fsdiogo - sorry, but but unclear on this, what do you mean a "custom webpack config", I didn't think webpack worked without a config. Am I assuming i can place this at the top level of the webpack config, or does it somehow need passing to UglifyJS.

@fsdiogo
Copy link
Contributor

fsdiogo commented Jun 11, 2018

Ignore the custom part, I was referring to your webpack config.

You have to pass the compress option to UglifyJS.

Check this comment for reference, but if you're using js-ipfs@0.29.0 or higher you can just pass this option to Uglify:

compress: {
	unused: false
}

@mitra42
Copy link

mitra42 commented Jun 12, 2018

Thanks - for the pointer to that comment, so its still unclear whether if you do not explicitly use Uglify (i.e. just use "webpack --mode production") you still need to add.

new UglifyJsPlugin({
    compress: { 
        unused: false
    }
}

@mitra42
Copy link

mitra42 commented Jun 12, 2018

Without that code (I don't actually use uglify) then I see a lot of Error: Callback was already called. , its hard to tell where these are coming form (since the code is minified) but they don't occur in the non-minified version.

Its possible of course that I'm doing something wrong, but it would be interesting if anyone else is seeing these on the minified IPFS.

@fsdiogo
Copy link
Contributor

fsdiogo commented Jun 13, 2018

Thanks - for the pointer to that comment, so its still unclear whether if you do not explicitly use Uglify (i.e. just use "webpack --mode production") you still need to add.

If you use webpack --mode production you will still need to add it, because the webpack production mode uses UglifyJS, which in turn has the unused option set to true by default: https://webpack.js.org/concepts/mode/.

That's why you are seeing the Error: Callback was already called, check my PR enabling compression, I talked about it there.

dryajov pushed a commit to dryajov/js-multiaddr that referenced this issue Jun 21, 2018
Replace constructor.name with instanceof. This is part of ongoing effort to properly
fix ipfs/js-ipfs#1131.
@mitra42
Copy link

mitra42 commented Jul 11, 2018

Has anyone else been able to follow the notes here and get Uglify to work ?
Adding

const UglifyJsPlugin = require('uglifyjs-webpack-plugin');
...
    plugins: [
        new UglifyJsPlugin({
            compress: {
                unused: false
            }
        })
    ]

to webpack.config results in

UglifyJs Plugin Invalid Options

options['compress'] is an invalid additional property

ValidationError: UglifyJs Plugin Invalid Options

@alanshaw
Copy link
Member

alanshaw commented Jul 12, 2018

Try:

const UglifyJsPlugin = require('uglifyjs-webpack-plugin');
...
    plugins: [
        new UglifyJsPlugin({
            uglifyOptions: {
              compress: {
                  unused: false
              }
            }
        })
    ]

@mitra42
Copy link

mitra42 commented Jul 12, 2018

Thanks @alanshaw that worked, It would probably a good idea to add that configuration to https://github.com/ipfs/js-ipfs/blob/master/examples/browser-webpack/webpack.config.js .

@daviddias
Copy link
Member

@mitra42 what about submitting that as a PR? ;)

@mitra42
Copy link

mitra42 commented Jul 13, 2018

Sure ... about half way through doing this, I realized that the browser-webpack example is actually a browser-webpack-react example, and while it works with the builtin webpack server, it wont actually compile with the usual ways of building a bundle that can be included in a project, webpack --mode development or webpack --mode production.

Does this need splitting into two examples, with one just showing how people can include IPFS in their (compressed webpack) projects without hitting this bug ?

@alanshaw
Copy link
Member

Yes, a webpack example without framework will allow people to use it as a base and add their own framework. Many people will be using webpack+react, but not eveyone!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet