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

Control passed to handler before completing middleware chain #98

Closed
theycallmeswift opened this issue Sep 26, 2018 · 4 comments
Closed

Comments

@theycallmeswift
Copy link

Hey, folks --

I feel like I must be doing something silly here, but I'm experiencing an issue where the middleware chain is not completely resolved before my code passes control to the final handler. Let me know if this is something I'm doing or an actual bug, thanks!

Setup:

Expected Output:

Middleware #1: Triggered
Middleware #1: Next
Middleware #2: Triggered
Middleware #2: Next
Middleware #3: Triggered
Middleware #3: Next
Finished

Actual Output:

Middleware #1: Triggered
Middleware #1: Next
Middleware #2: Triggered
Middleware #2: Next
Middleware #3: Triggered
Finished
Middleware #3: Next

Code:

const compose = require('koa-compose');

/**
 * Simple App Structure.
 */
class App {

  constructor () {
    this.middleware = [];
  }

  use (fn) {
    if (typeof fn !== 'function') {
      throw new TypeError('middleware must be a function!');
    }

    this.middleware.push(fn);

    return this;
  }

  route (handler) {
    let fn = compose(this.middleware);
    return fn(this).then( () => this.handleRequest(handler) );
  }

  handleRequest (handler) {
    return handler();
  }
}

let app = new App();

/**
 * Setup Middleware
 */
app.use( (ctx, next) => {
  console.log("Middleware #1: Triggered");
  console.log("Middleware #1: Next");
  next();
});

app.use( (ctx, next) => {
  console.log("Middleware #2: Triggered");
  console.log("Middleware #2: Next");
  next();
});

app.use( (ctx, next) => {
  console.log("Middleware #3: Triggered");
  setTimeout( () => {
    console.log("Middleware #3: Next");
    next();
  }, 2000);
});

app.route(() => console.log("Finished") );
@PlasmaPower
Copy link
Contributor

You need to return a promise if you do asynchronous activities, which includes calling next IIRC (it's been a while since I've worked with Koa).

Here's a working example: https://gist.github.com/1f0670e88911cac546dc80548bca1865

@theycallmeswift
Copy link
Author

@PlasmaPower your example does work. I'm struggling to understand why though. This feels like Promise black magic haha.

Thoughts on this being an antipattern? My experience with callbacks is that you need to call next before moving on.

@PlasmaPower
Copy link
Contributor

You're probably thinking of the express model. In Koa, you almost always return a promise.

@fl0w
Copy link
Contributor

fl0w commented Sep 27, 2018

Expanding on @PlasmaPower's answer: Promises need to be chain returned otherwise any of them will resolve and Koa flushes the response. All next calls return a promise guaranteed because of how compose works - meaning that even if you call next expecting it will be a normal function it's actually a promise.

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

4 participants