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

Weird behavior with fastify.register, async/await and point-of-view #267

Closed
iCyberon opened this issue Sep 20, 2017 · 11 comments
Closed

Weird behavior with fastify.register, async/await and point-of-view #267

iCyberon opened this issue Sep 20, 2017 · 11 comments
Labels
question General question about the project

Comments

@iCyberon
Copy link

iCyberon commented Sep 20, 2017

Hey, during my initial tests I've encountered a weird issue related to fastify.register and async/await. Below is a simplified version of what I'm doing.

app.js

let loader = async () => {
  try {
    await require('./plugins')(fastify)
    // ...
    // ...
    fastify.get('/', (req, reply) => {
      reply.view('/templates/index.ebs', { text: 'text' })
    })
    fastify.listen(3000)
  } catch (err) {
    log.fatal(err)
    throw err
  }
}

loader()

plugins.js is a wrapper to register plugins

const util      = require('util')
const fs        = require('fs')
const path      = require('path')

const readdir   = util.promisify(fs.readdir)

module.exports = async function(fastify) {
  const plugins = (await readdir(path.join(__dirname, 'plugins')))

  for (const name of plugins) {
    const plugin = require(path.join(__dirname, 'plugins', name));
    fastify.register(plugin.fn, plugin.opts)
  }
}

/plugins/views.js is loaded by plugins.js

module.exports.fn = require('point-of-view')
module.exports.opts = {
  engine: {
    ebs: require('ebs')
  }
}

This doesn't work and I get an exception

Exception has occurred: TypeError
TypeError: reply.view is not a function
    at Store.fastify.get [as handler] (/Volumes/Test/Fastify/app.js:46:19)
    at State.preHandlerCallback (/Volumes/Test/Fastify/node_modules/fastify/lib/handleRequest.js:117:27)
    at ResultsHolder.release (/Volumes/Test/Fastify/node_modules/fastseries/series.js:99:22)
    at Immediate.series (/Volumes/Fastify/node_modules/fastseries/series.js:39:14)
    at runCallback (timers.js:790:39)
    at tryOnImmediate (timers.js:743:5)
    at processImmediate [as _immediateCallback] (timers.js:714:5)

But if I replace const plugins = await readdir(path.join(__dirname, 'plugins')) with a sync version const plugins = fs.readdirSync(path.join(__dirname, 'plugins')) everything works as expected.

@iCyberon
Copy link
Author

Here's a smaller code to replicate the issue.

const fastify = require('fastify')();

const reg = async function(fastify) {
    _ = await 1;
    fastify.register(require('point-of-view'), {
        engine: {
            handlebars: require('handlebars')
        }
    });
}

_ = (async function() {
    await reg(fastify);
    fastify.get('/', (req, reply) => {
        reply.view('/templates/index.hbs', { text: 'text' })
    })
    fastify.listen(3000)
})();

@mcollina
Copy link
Member

What you are doing is clearly unsupported. I am a bit lost on why you would like to load fastify using async/await, maybe if you give us a little bit more context we can help.

We also accept PRs on all our modules. In fact, I think that the next version of avvio (I’ll release that tomirrow), might fix this.

@delvedor delvedor added the question General question about the project label Sep 20, 2017
@iCyberon
Copy link
Author

iCyberon commented Sep 20, 2017

It's not about loading fastify in async, it's a consequence of trying to make the code cleaner.
Let's assume I have functions like

  • async loadPlugins()
  • async loadMiddlewares()
  • async loadUtilities()

They are async because they need to scan directories, get info from db, etc... Basically I have three options - doing everything synchronously, using promises w/ chaining, using async/await. The last one makes code cleaner. Because of this I need to wrap fastify init in an async function.

Hope it's clear what are my intentions

@delvedor
Copy link
Member

Hi!
I suggest you to checkout our plugins guide and our plugin documentation.
There is no need to use async/await as your are doing, you could easily use the register api, which is designed to handle the asynchronous bootstrapping of an application :)
If you are curious and you want to know how we do that, checkout avvio!

@iCyberon
Copy link
Author

There is no need to use async/await as your are doing, you could easily use the register api, which is designed to handle the asynchronous bootstrapping of an application :)

@delvedor, I might have been missing something. Just to clarify, my use-case is something like this.
App needs to connect to db and fetch some configs (not just once), those configs are used to instantiate plugins (list of plugins I need to load) and add some decorators like config (again, the content is fetched from db). So my options are:

  • load everything at once from database first then instantiate fastify after everything is loaded
  • lazy way, fetch when needed. (if I'm trying to add decorators fetch config from db and loop)

I picked the second option, as a result my loadDecorators is an async function (I use await to fetch results from db) and that means that I need to wait until loadDecorators is done which means my loader is also async 🤷‍♂️

Did I miss something? Because it looks like it can't be done in fastify without droping async/await. Anyway, thanks for response. Will check avvio.

P.S. I still don't understand why loading fastify inside async/await is not supported, some tips?

@mcollina
Copy link
Member

mcollina commented Sep 21, 2017

@iCyberon a problem in your code snippet is that register as a callback, which you are currently ignoring, and it ends up with two competing system (fastify/avvio and async/await) to asynchronously control the startup of the application. This is exercising some code path that we have not tested, and it probably has a bug.

If you would like to send a PR to help supporting your use case, we would be very happy.


You should wrap your lookup logic of plugins, decorators and middlewares in plugins. Just call next() when each step is done.

@delvedor
Copy link
Member

Closing this, if you have more questions feel free to reopen :)

@rodrigogs
Copy link

rodrigogs commented Sep 27, 2017

I'm having the same issue, I think...

Here's my sippet:

  const registerRoutes = () => new Promise((resolve, reject) => {
    app.register(router, {}, (err) => {
      if (err) return reject(err);
      app.Router.route(routes);
      resolve();
    });
  });

Inside the callback app.Router is undefined, but if I execute it outside of a Promise it behaves normally... any idea why?

@delvedor
Copy link
Member

@rodrigogs can you paste also the router code?
I don't understand the need of the promise.

Inside the callback you can't access app.Router because the register calls are encapsulated, checkout our plugins guide, it might help :)

@rodrigogs
Copy link

How not?

  app.register(router, {}, (err) => {
    if (err) throw err;
    app.Router.route(routes);
  });

This example works perfectly. It just doesn't work when executed inside a promise... wich makes no sense to me.

Router code is here: https://github.com/rodrigogs/fastify-router

@rodrigogs
Copy link

I already came with a solution, sinse the promise I was using were useless. But I still wonder why the same code wont work inside a promise. :/

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

No branches or pull requests

4 participants