[:zh]一个不正确的 TypeScript 类型定义引发的血案[:en]An incident caused by an incorrect TypeScrpit type definition of Mongoose[:]

[:zh]Mongoose 的 TypeScript 类型定义并不完全正确！

昨天晚上在 bgs 研究了（或者说调试了）XGS 写的 nodejs 服务器项目。他在用 koa 作为服务器，mongodb 作为数据存储，写一个不知道什么项目（似乎名为「写作者平台」/ “Write’s Platform”）。反正当时还处于很初级的阶段，只有两个路由，一个 get，一个 post，后者是用来向 mongodb 提交一篇文章的。在测试过程中，他发现了一个问题——他在保存文章的回调中更改了响应的内容（如下），但是实际测试得到的响应内容却是 koa 默认的 OK。

router.post('/path', (ctx, next) => {
    const article = new Article( /* something */ );
    article.save((err, article) => {
      if (err){
        ctx.body = err;
      } else {
        ctx.body = "success";
      }
    });
})

起初我发现，他的代码中，router.post('/path', (ctx, next) => { /* body */ });的/* body */这段中并没有返回 next()，这应该会导致异步的回调没有被阻塞。

接着我想，可能又是 koa 的版本搞混了什么的，因为 koa v2，koa router 什么的版本管理都很混乱，长期处于虽然大家都想使用最新版，但是默认的却一直是旧版的情况。后来又出现了 koa 是新版，router 是旧版的情况，曾经我为别人解决过这个问题。

但是这次却不是。并且把返回 promise 切换为使用 async/await之后（如下），情况依然没有好转。

router.post('/path', (ctx, next) => {
    const article = new Article( /* something */ );
    return article.save((err, saved) => {
      if (err){
        ctx.body = err;
      } else {
        console.log(saved);
        ctx.body = "success";
      }
      // or `return next();` here.
    }).then((thing) => {
        console.log(thing);
        return next();
    });
})

（过程中我们发现这个传给回调的 thing居然是undefined，这是个伏笔）

渐渐地我被各种函数的签名（比如究竟应该返回一个什么，是 next() 还是一个 promise，而 next 的类型又是什么？）搞得烦躁起来。于是我提议使用 TypeScript 来辅助检查。

我们开始怀疑 article.save这个函数。因为在 koa 官方（仅有）的、涉及到等待异步动作的 router 代码样例里，完全类似的代码是可以工作的：

router.get(
  '/users/:id',
  function (ctx, next) {
    return User.findOne(ctx.params.id).then(function(user) {
      ctx.user = user;
      return next();

    });
  },
  function (ctx) {
    console.log(ctx.user);
    // => { id: 17, name: "Alex" }
  }
);

于是我们着重检查了 mongoose 的 TypeScript 类型定义文件。这个 article是Article的实例，而Article实际上是 mongoose 动态构造出来的一个类（应该是通过了某种元编程技术）。在类型定义中是这么反映的：

interface Model<T extends Document> extends NodeJS.EventEmitter, ModelProperties {
    new(doc?: Object): T;
    /* other things */
}

而 article 就是那个<T extends Document>，而 Document的save方法是这么写的：

    /**
     * Saves this document.
     * @param options options optional options
     * @param options.safe overrides schema's safe option
     * @param options.validateBeforeSave set to false to save without validating.
     * @param fn optional callback
     */
    save(options?: SaveOptions, fn?: (err: any, product: this, numAffected: number) => void): Promise<this>;
    save(fn?: (err: any, product: this, numAffected: number) => void): Promise<this>;

（取自 npm install @types/mongoose的结果，不知为何与 DefinitelyTyped 上的有所不同。）看到这，特别是Promise<this>，我就确信了，这个函数没有问题。可是上面返回的thing又确乎是一个undefined，难道是我们的使用姿势有问题？

万般无奈之中，TypeScript 也不靠谱之际，我们只能诉诸源码了。鉴于 mongoose 的代码风格，直接搜索 .prototype.save就能找到。它实际上是在这里，但是看到这里让人更崩溃了：

Model.prototype.save = function(options, fn) {
  if (typeof options === 'function') {
    fn = options;
    options = undefined;
  }

  if (!options) {
    options = {};
  }

  if (fn) {
    fn = this.constructor.$wrapCallback(fn);
  }

  return this.$__save(options, fn);
};

它返回的是this.$__save的结果，但这个函数并没有返回值！日志显示，这里的this就是这个article实例（一个Document）。然而线索到这里似乎断了。怎么能凭空从一个没有返回值的函数返回一个 promise 呢？

我们又想到了一招：查看调用栈。但是 node 的调试器在这时却不好使了。我们只能用原始粗暴的console.trace手动搞出调用栈。虽然没有直接的线索，这个调用栈给了我们一些提示——save并不是直接调用的，而是经过了某种hook的封装。

这时，我想到了直接console.log(article.save)，它居然是一个wrappedPointCut出来的结果！再在代码里搜索wrappedPointCut就非常接近真相了。关键的代码在这里：

model.prototype[pointCut] = (function(_newName) {
      return function wrappedPointCut() {
        var Promise = PromiseProvider.get();

        var _this = this;
        /* something omitted for simplicity */
        var promise = new Promise.ES6(function(resolve, reject) {
          args.push(function(error) {
            if (error) {
              /* something omitted for simplicity */
              reject(error);
              return;
            }
            $results = Array.prototype.slice.call(arguments, 1);
            resolve.apply(promise, $results);
          });
          _this[_newName].apply(_this, args);
        });
        if (fn) {
          /* something omitted for simplicity */
          return promise.then(
            function() {
              process.nextTick(function() {
                fn.apply(null, [null].concat($results));
              });
            },
            function(error) {
              process.nextTick(function() {
                fn(error);
              });
            });
        }
        return promise;
      };
    })(newName);

注意到如果没有fn，那么会在第 33 行返回第 7 行定义的 promise，如果有fn，就会直接在第 21 行返回接续之前的 promise 的另一个 promise，这个接续（.then()）里面利用了fn处理了结果，却没有返回。也就是说，这时候的返回值是Promise<undefined>而不是Promise<this>——我们被这样的 TypeScript 类型定义坑惨了！如果它一开始就告诉我们会这样，那该多好呀，就像这样：

    save(options: SaveOptions, fn: (err: any, product: this, numAffected: number) => void): Promise<undefined>;
    save(fn: (err: any, product: this, numAffected: number) => void): Promise<undefined>;
    save(options?: SaveOptions): Promise<this>;
    save(): Promise<this>;

2017 年 5 月 4 日

[:en]The TypeScript type definition of Mongoose are not fully correct!

I inspected (or debugged) a nodejs server project of Little Gengsha‘s, where he’s using koa as the server and mongodb as the data store. The project seems called “Writer’s Platform”, which seemed very primitive then. There were only two routes, a get, a post, the latter of which is used to submit an article to mongodb. During the test he discovered a problem — the body of the response should be changed in the callback of the save function, but the action response is “OK”, which is the default response of koa.

router.post('/path', (ctx, next) => {
    const article = new Article( /* something */ );
    article.save((err, article) => {
      if (err){
        ctx.body = err;
      } else {
        ctx.body = "success";
      }
    });
})

At the first sight I discovered thatnext() is not returned in /* body */ of router.post('/path', (ctx, next) => { /* body */ });This may cause the response to return early before the save action calls back. But simply return a next() did not help.

Then I thought it could be due to the chaos of the versions of the koa ecosystem. The versions of koa and koa-router are all mixed. They both defaulted to the old version while people would like to use the new, under-development version. Koa advanced afterwards, while koa-router still defaults to the older version. I once encountered a problem caused by such incompatibility and solved it for someone.

But this time it’s not the case. We switched from promise to async/await, but the problem persists.

router.post('/path', (ctx, next) => {
    const article = new Article( /* something */ );
    return article.save((err, saved) => {
      if (err){
        ctx.body = err;
      } else {
        console.log(saved);
        ctx.body = "success";
      }
      // or `return next();` here.
    }).then((thing) => {
        console.log(thing);
        return next();
    });
})

(However, we discovered that  thing is undefined, which turns out to be the key)

I was then upset with the signatures of the various functions (like what should be returned in the callback of router.get(), or what is the type of next), so I proposed using TypeScript for auxiliary checks.

We began to suspect that  article.save is not working as expected. The function, which is similar to the only sample code from koa regarding async operations, is expected to work as demonstrated by the doc：

router.get(
  '/users/:id',
  function (ctx, next) {
    return User.findOne(ctx.params.id).then(function(user) {
      ctx.user = user;
      return next();

    });
  },
  function (ctx) {
    console.log(ctx.user);
    // => { id: 17, name: "Alex" }
  }
);

So we specially inspected the TypeScript type definition of mongoose, in hope of finding any misuse. The  article is an instance of Article, while Article is actually a class (not only a instance, but also a class itself) dynamically constructed by mongoose. It is reflected in the type system:

interface Model<T extends Document> extends NodeJS.EventEmitter, ModelProperties {
    new(doc?: Object): T;
    /* other things */
}

and article is just the <T extends Document>， Document‘s savemethod is written as follows：

    /**
     * Saves this document.
     * @param options options optional options
     * @param options.safe overrides schema's safe option
     * @param options.validateBeforeSave set to false to save without validating.
     * @param fn optional callback
     */
    save(options?: SaveOptions, fn?: (err: any, product: this, numAffected: number) => void): Promise<this>;
    save(fn?: (err: any, product: this, numAffected: number) => void): Promise<this>;

(taken from the result of npm install @types/mongoose, which somehow differs from that on the github repo of DefinitelyTyped .)  Seeing the Promise<this>, I was convinced that our use of the function is valid. But the fact is the thing returned above is undefined. So are there any peculiarities?

Since TypeScript was not reliable then, we could only resort to the source code. Directly searching for  .prototype.save will yield us the relevant code. It’s actually here. But seeing it makes me more desperate:

Model.prototype.save = function(options, fn) {
  if (typeof options === 'function') {
    fn = options;
    options = undefined;
  }

  if (!options) {
    options = {};
  }

  if (fn) {
    fn = this.constructor.$wrapCallback(fn);
  }

  return this.$__save(options, fn);
};

It returns the result of this.$__save, but that function does not return actually! How could it return a promise from a function that returns nothing？

We thought of another ultimate measure: inspecting the call stack。We used console.trace for it. Although no direct clue shown, it still gave us some hint——save is not called directly, but wrapped with some hook.

Suddenly, I thought about directly console.log(article.save), which told us that article.save is actually the result of a wrappedPointCut function! Searching for wrappedPointCut in the code almost revealed the answer. The crucial part is here:

model.prototype[pointCut] = (function(_newName) {
      return function wrappedPointCut() {
        var Promise = PromiseProvider.get();

        var _this = this;
        /* something omitted for simplicity */
        var promise = new Promise.ES6(function(resolve, reject) {
          args.push(function(error) {
            if (error) {
              /* something omitted for simplicity */
              reject(error);
              return;
            }
            $results = Array.prototype.slice.call(arguments, 1);
            resolve.apply(promise, $results);
          });
          _this[_newName].apply(_this, args);
        });
        if (fn) {
          /* something omitted for simplicity */
          return promise.then(
            function() {
              process.nextTick(function() {
                fn.apply(null, [null].concat($results));
              });
            },
            function(error) {
              process.nextTick(function() {
                fn(error);
              });
            });
        }
        return promise;
      };
    })(newName);

Without fn， a promise defined on line 7 will be returned on line 33, while with afn in the parameter, the promise defined would be chained with a then on line 21, which returns nothing — In this case, the return value should be Promise<undefined> instead of Promise<this>—— We were fooled by such sloppy type definition! How I wish it could be annotated correctly from the beginning, just like this:

    save(options: SaveOptions, fn: (err: any, product: this, numAffected: number) => void): Promise<undefined>;
    save(fn: (err: any, product: this, numAffected: number) => void): Promise<undefined>;
    save(options?: SaveOptions): Promise<this>;
    save(): Promise<this>;

May, 4, 2017

[:]