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

The context can not work with _.cloneDeep #1600

Open
xiedacon opened this issue Oct 15, 2021 · 4 comments
Open

The context can not work with _.cloneDeep #1600

xiedacon opened this issue Oct 15, 2021 · 4 comments

Comments

@xiedacon
Copy link

xiedacon commented Oct 15, 2021

Test codes:

const _ = require('lodash');
const Koa = require('koa');

const app = new Koa();

app.use((ctx) => {
    _.cloneDeep(ctx);

    ctx.body = {
        test: 'test',
    };
});

app.listen(3000);
curl http://127.0.0.1:3000/
  TypeError: Cannot read property 'length' of undefined
      at Object.length (/Users/admin/Downloads/test/node_modules/delegates/index.js:72:24)
      at isArrayLike (/Users/admin/Downloads/test/node_modules/lodash/lodash.js:11401:46)
      at keys (/Users/admin/Downloads/test/node_modules/lodash/lodash.js:13375:14)
      at baseGetAllKeys (/Users/admin/Downloads/test/node_modules/lodash/lodash.js:3094:20)
      at getAllKeys (/Users/admin/Downloads/test/node_modules/lodash/lodash.js:5916:14)
      at baseClone (/Users/admin/Downloads/test/node_modules/lodash/lodash.js:2726:39)
      at /Users/admin/Downloads/test/node_modules/lodash/lodash.js:2733:34
      at arrayEach (/Users/admin/Downloads/test/node_modules/lodash/lodash.js:530:11)
      at baseClone (/Users/admin/Downloads/test/node_modules/lodash/lodash.js:2727:7)
      at /Users/admin/Downloads/test/node_modules/lodash/lodash.js:2733:34

I found it was caused by the code here https://github.com/koajs/koa/blob/19f3353e5da6a0d52fb968a9429523585e42a288/lib/context.js

The simple code is like this:

const _ = require('lodash');

const contextProto = {};
// delegates
Object.defineProperty(contextProto, 'length', {
    get: function () {
        return this.response.length;
    },
    set: function (val) {
        this.response.length = val;
    },
});

// new App()
const app = {
    context: Object.create(contextProto),
};

// new ctx
const ctx = Object.create(app.context);
ctx.app = app;
ctx.response = { length: 1 };

console.log(_.cloneDeep(ctx));
// console.log(_.cloneDeep(app));

When execute _.cloneDeep(ctx), app will also be cloned. But app.context can not be clone, because lodash needs the length proproty to decide whether obj is array-like https://github.com/lodash/lodash/blob/2da024c3b4f9947a48517639de7560457cd4ec6c/isArrayLike.js

It also happens on request and response.

I think maybe we should do some special operations for length property, just like this:

const _ = require('lodash');

const contextProto = {};
// delegates
Object.defineProperty(contextProto, 'length', {
    get: function () {
        return this.response?.length;
    },
    set: function (val) {
        if (this.response) {
            this.response.length = val;
        }
    },
});

// new App()
const app = {
    context: Object.create(contextProto),
};

// new ctx
const ctx = Object.create(app.context);
ctx.app = app;
ctx.response = { length: 1 };

console.log(_.cloneDeep(ctx));
// console.log(_.cloneDeep(app));
@dead-horse
Copy link
Member

Is there a specific usage scenario that requires clone ctx?

@xiedacon
Copy link
Author

xiedacon commented Nov 14, 2021

In our case, it happens when we use async_hooks and sequelize at the same time.

At the async_hooks part, we need to use it to pass the ctx, so there is no need to add the ctx parameter to every function.

const { AsyncLocalStorage } = require('async_hooks')

const Koa = require('koa')

const localStorage = new AsyncLocalStorage();
const app = new Koa();

app.use((ctx, next) => {
  localStorage.run({ ctx }, next)
});

app.use((ctx) => {
  ctx.body = fn1()
});

const fn1 = () => fn2()
const fn2 = () => fn3()
const fn3 = () => {
  const { ctx } = localStorage.getStore()

  // do something

  return ctx.query
}

app.listen(3000);

At the sequelize part, we need to use the dialectModule options to pass our own mysql driver to sequelize.

There are two important things:

  1. There is an timer on the mysql driver.
  2. The sequelize will use _.cloneDeep to copy options before querying sql. https://github.com/sequelize/sequelize/blob/main/lib/dialects/abstract/connection-manager.js#L21
const { Sequelize } = require('sequelize')

const dialectModule = {
  intervalId: setInterval(() => {
    console.log('some thing')
  }, 1000)
}

const sequelize = new Sequelize({
  dialectModule
})

// It will call _.cloneDeep() to dialectModule.
sequelize.query('SELECT 1 = 1')

In the end, our code looks like the following:

const { AsyncLocalStorage } = require('async_hooks')

const Koa = require('koa')
const { Sequelize } = require('sequelize')

const localStorage = new AsyncLocalStorage();
const app = new Koa();

app.use((ctx, next) => {
  localStorage.run({ ctx }, next)
});

app.use(async (ctx) => {
  ctx.body = await someController()
});

const someController = async () => someService()
const someService = async () => someModel()
const someModel = async () => {
  const { ctx } = localStorage.getStore()
  const sequelize = new Sequelize({
    dialectModule: {
      intervalId: setInterval(() => {
        console.log('some thing')
      }, 1000)
    }
  })

  console.log(ctx.query)

  // TypeError: Cannot read property 'length' of undefined
  return await sequelize.query('SELECT 1 = 1')
}

app.listen(3000);

It will be thrown when querying sql, even if the async_hooks has no business with it.

The reason is

  1. When using AsyncLocalStorage, it will store the ctx in the Symbol(kResourceStore) property of each timer.
  2. When querying sql, the sequelize will use _.cloneDeep to copy options.
  3. Copying options means copying dialectModule, means copying timer, means copying ctx, and finally equals _.cloneDeep(ctx).

@xiedacon
Copy link
Author

An example without sequelize.

const { AsyncLocalStorage } = require('async_hooks')

const _ = require('lodash')
const Koa = require('koa')

const localStorage = new AsyncLocalStorage();
const app = new Koa();


app.use((ctx, next) => {
  localStorage.run({ ctx }, next)
});

app.use(async (ctx) => {
  ctx.body = await someController()
});

const someController = async () => someService()
const someService = async () => someModel()
const someModel = async () => {
  // TypeError: Cannot read property 'length' of undefined
  _.cloneDeep(setTimeout(() => {}, 1000))

  return { some: 'thing' }
}

app.listen(3000);

@siakc
Copy link

siakc commented Dec 22, 2023

The reason for this error is that length is a delegated property of ctx. So when lodash tries to read it to see if the object has a length, delegate getter is actually called and I guess as the properties are not fully cloned it fails.

I guess this is fixed in newer version of lodash as it checks to see if the property is not a function first.

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

No branches or pull requests

3 participants