Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError: Cannot read property 'length' of undefined #5896

Closed
cbratschi opened this issue Dec 11, 2017 · 6 comments
Closed

TypeError: Cannot read property 'length' of undefined #5896

cbratschi opened this issue Dec 11, 2017 · 6 comments

Comments

@cbratschi
Copy link

Do you want to request a feature or report a bug?
bug

What is the current behavior?
Mongoose 4.13.6 crashes where 4.11.13 was working fine.

If the current behavior is a bug, please provide the steps to reproduce.

Our schemas are use child schemas. Mongoose accesses a length property which does not exist.

This is the stack trace:

TypeError: Cannot read property 'length' of undefined
at Mongoose._applyPlugins (Z:\git\pharos-node-server\node_modules\mongoose\lib\index.js:486:40)
at Mongoose._applyPlugins (Z:\git\pharos-node-server\node_modules\mongoose\lib\index.js:487:10)
at Mongoose._applyPlugins (Z:\git\pharos-node-server\node_modules\mongoose\lib\index.js:487:10)
at Mongoose.model (Z:\git\pharos-node-server\node_modules\mongoose\lib\index.js:396:10)

Are there breaking changes between those versions concerning child schemas?

What is the expected behavior?
Should not crash.

Please mention your node.js, mongoose and MongoDB version.
Node v8.4.0 and MongoDB 3.8.

@cbratschi
Copy link
Author

This change introduced the crashing code:

af73f64#diff-6d186b954a58d5bb740f73d84fe39073

@vkarpov15
Copy link
Collaborator

Do you have a code sample that reproduces this? Shouldn't be any way for a schema to not have a childSchemas array...

@vkarpov15 vkarpov15 added this to the 4.13.8 milestone Dec 13, 2017
@vkarpov15
Copy link
Collaborator

Also, no intentional breaking changes between those versions.

@vkarpov15 vkarpov15 added the needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue label Dec 13, 2017
@cbratschi
Copy link
Author

Here are some code snippets of the used schemes:

/**
 * Scene schedule.
 */
const EVT_SHOW   = 'show';
const EVT_HIDE   = 'hide';
const EVT_REPORT = 'report';

const SceneEvents = new Schema({
    event: { type: String, enum: [ EVT_SHOW, EVT_HIDE, EVT_REPORT ], required: true },
    schedule: {
        start: { type: Number },
        end: { type: Number },
        hour: { type: String },
        minute: { type: String },
        second: { type: String },
        dayOfWeek: { type: String },
        date: { type: String },
        month: { type: String },
        year: { type: String },
        timezone: { type: String, required: true }
    },

    //optional fields
    data: { type: Object }
});

/**
 * DisplayScene
 */
const displaySceneFields = {
    //metadata
    name: { type: String, required: true },
    description: { type: String },

    //creation & update
    creation: { type: Date, default: Date.now, required: true },
    update: { type: Date, default: Date.now, required: true },

    //scene data
    scene: { type: String, required: true },
    theme: { type: String },

    //custom data
    data: { type: Object },

    //schedule
    scheduled: { type: Boolean },
    events: [ SceneEvents ]
};
const DisplayScene = new Schema(displaySceneFields);

/**
 * ProjectDataScenes fields.
 */
const globalSceneFields = clone(displaySceneFields);

delete globalSceneFields.scheduled; //display bound scene only

globalSceneFields.active = { type: Boolean };
globalSceneFields.displays = [{
    display: { type: ObjectId, required: true },
    type: { type: String, enum: DISPLAY_TYPES, required: true }
}];

const GlobalScene = new Schema(globalSceneFields);
const globalScenesFields = {
    data: {
        //scenes
        scenes: [ GlobalScene ]
    }
};

const GlobalScenes = projectsData.createProjectDataSchema(globalScenesFields);
const GLOBAL_SCENES_NAME = 'scenes';

//crashes here
const ScenesModel = mongoose.model('ProjectDataScenes', GlobalScenes, 'projectdatas');

The clone module is being used and createProjectDataSchema() is:

const projectDataFields = {
    //project (indexed)
    project: { type: ObjectId, ref: 'Project', required: true },

    //name (indexed)
    name: { type: String, trim: true, required: true },

    //creation & update
    creation: { type: Date, default: Date.now, required: true },
    update: { type: Date, default: Date.now, required: true },

    //data
    data: { type: Schema.Types.Mixed, required: true }
};

/**
 * Create a customized schema.
 */
function createProjectDataSchema(fields) {
    const data = clone(projectDataFields);

    //copy fields
    if (fields) {
        for (let key of Object.keys(fields)) {
            data[key] = fields[key];
        }
    }

    return new Schema(data, { minimize: false }); //Note: don't remove empty objects
}

I am using a template schema and customized sub-schemas where the data field contains typed fields. But I guess the [ GlobalScene ] causes the issue.

@vkarpov15
Copy link
Collaborator

I narrowed this down to a subtle bug in clone: pvorb/clone#88 . Wish there was something I could do to make this work with mongoose, but until that PR is merged every JS class that declares properties on the prototype does not work with clone, including all of mongoose.

As a workaround, I'd recommend you use lodash.clonedeep, it should be a drop-in replacement for clone and doesn't suffer from this issue. Although in this case I don't think you even need deep cloning, so you might just try doing const data = Object.assign({}, projectDataFields) to do a shallow clone without using any libs.

@vkarpov15 vkarpov15 removed this from the 4.13.8 milestone Dec 21, 2017
@vkarpov15 vkarpov15 added won't fix and removed needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue labels Dec 21, 2017
@cbratschi
Copy link
Author

Thanks a lot for the analysis. Object.assign() is enough in my cases.

Happy holidays and keep up the great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants