Clone breaking v8 somehow #106
Comments
|
Your steps to reproduce don’t work: |
|
I now changed your sample script to the following: const Sequelize = require('sequelize');
const clone = require('clone');
async function test() {
const sequelize = new Sequelize(
'database-name',
'user',
'password',
{ host: 'ip', dialect: 'postgres' }
);
const Dummy = sequelize.define('dummy', {
id: {
allowNull: false,
autoIncrement: true,
primaryKey: true,
unique: true,
type: Sequelize.BIGINT
}
});
await Dummy.sync();
const dummy = new Dummy({id: 1});
await dummy.save();
const clonedDummy = clone(dummy);
console.log(clonedDummy);
}
test();This works flawlessly with Node.js v8.x, but doesn’t work with Node.js v10.16.11, failing with the following exception: So the root cause is clear. Sequelize opens a TCP connection, which, of course, cannot be cloned because it's a resource handle. In order to make the object clonable, remove the reference to the TCP connection first, e.g. by deleting the This code works: const Sequelize = require('sequelize');
const clone = require('clone');
async function test() {
const sequelize = new Sequelize(
'clone_issue_106',
'postgres',
'postgres',
{host: 'localhost', dialect: 'postgres'}
);
const Dummy = sequelize.define('dummy', {
id: {
allowNull: false,
autoIncrement: true,
primaryKey: true,
unique: true,
type: Sequelize.BIGINT
}
});
await Dummy.sync();
const dummy = new Dummy({id: 1});
await dummy.save();
delete dummy._modelOptions;
const clonedDummy = clone(dummy);
console.log(clonedDummy);
}
test();I'm closing this, since I do not consider this a bug. Feel free to re-open if you do not agree. |
|
Just checked with Node v12.7.0 and got the same |
|
@pvorb So, do you think it is an issue with clone (and therefore we should open it again) or should we investigate a little more and open a ticket elsewhere? I have never seen v8 cracking like this. In a few days I will be able to investigate it further, but I want to know if you think it's worthwhile. |
|
Well, in order to clone the TCP connection, I don't think there is an issue with Node.js, since it is calling into native code. Native objects cannot be cloned using |
|
Just hit this same issue. A variation of #93 could people to avoid this (kind of) error where they are not in control of the rest of the code which calls clone. reproduction: |
|
PR added for extending clone, and basic crash prevention. |
Error
What I was trying to do
Inside
contextTreethere is a sequelize record instance. I was trying to clone it.What I have done to try to find the root cause
I isolated the error trying to clone its properties in isolation (
require('clone')(contextTree.a),require('clone')(contextTree.b), etc...) and could verify that the problem resides somewhere in the sequelize reference. However, I'm just a shitty developer without time to investigate more about its root causes. :(Steps to reproduce
Conclusion
The library shouldn't be liable to what is being cloned, but I find it bizarre that it breaks v8. I don't think it was suppose to do that even while cloning adverse and complex objects. There is something really wrong that I wasn't able to pin down. I think it is worth investigating.
ENV
The text was updated successfully, but these errors were encountered: