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

Use AsyncLocalStorage to create a session #2074

Open
DaddyWarbucks opened this issue Sep 22, 2020 · 8 comments
Open

Use AsyncLocalStorage to create a session #2074

DaddyWarbucks opened this issue Sep 22, 2020 · 8 comments
Labels

Comments

@DaddyWarbucks
Copy link
Member

This snippet is a basic example using AsyncLocalStorage to create a session object that is available to any "nested" services. This could be useful in passing things like user, authentication, transactions, etc to these service calls nested N levels deep instead of having to "params drilling" (aka explicitly passing params from service to service).

const { AsyncLocalStorage } = require('async_hooks');
const session = new AsyncLocalStorage();

const startSessionAsync = async (context, next) => {
  if (session.getStore()) {
    context.session = session;
    return next();
  } else {
    console.log('starting session: ', context.path);
    return session.run(new Map(), async () => {
      session.getStore().set('time', new Date().getTime());
      context.session = session;
      return next();
    });
  }
};


const logSession = context => {
  console.log(`logging from ${context.path}:${context.type} hook: `, {
    time: context.session.getStore().get('time')
  });
  return context;
};

const callNestedService = async context => {
  const posts = await context.app.service('posts').find({
    query: { user_id: context.result.user_id }
   // Notice that we do not explicitly pass the `context.session` to the posts service
  });
  return context;
}

// Use the `startSessionAsync` in app.hooks. If no session is "active" (via session.getStore()) then a new session
// is created. But, if this service is being called "nested" then a session will already exist and is "continued" to
// this nested service
app.hooks({
  async: [startSessionAsync]
});

usersService.hooks({
  before: {
    all: [logSession]
  }
 after: {
    all: [callNestedService, logSession]
  }
});

postsService.hooks({
  before: {
    all: [logSession]
  }
 after: {
    all: [logSession]
  }
});

usersService.find();

// logging from api/users:before hook:  { time: 1600811184874 }
// logging from api/posts:before hook:  { time: 1600811184874 }
// logging from api/posts:after hook:  { time: 1600811184874 }
// logging from api/users:after hook:  { time: 1600811184874 }

You can see a full working example here: https://github.com/DaddyWarbucks/test-feathers-cls
And you can read more about AsyncLocalStorage here: https://nodejs.org/api/async_hooks.html#async_hooks_class_asynclocalstorage

Note that there are performance penalties for using AsynLocalStorage. A quick Google search will turn up some benchmarks, although none are very detailed.

It is also important to note that AsyncLocalStorage was released in Node v13 (with backports for later v12 versions). So this is a rather new feature. But, I also found https://github.com/kibertoad/asynchronous-local-storage which would be a good example for how to fallback to cls-hooked if we wanted to support back to Node v8.

@bitflower
Copy link
Contributor

Chimin‘ in...

@DaddyWarbucks
Copy link
Member Author

Related: #1881

@DaddyWarbucks
Copy link
Member Author

DaddyWarbucks commented Nov 12, 2020

Just throwing this in here as a potential way to handle this pre v5. I typed this out in response to another issue asking about setting up NewRelic and passing around that transaction, but then deleted it from there because it was more relevant here.

The current way hooks are handled, they are not in the same "execution context" for this to work. But we could create a mixin, or extend classes to make this work I believe. The following code is not tested and just an idea.

const { AsyncLocalStorage } = require('async_hooks');
const session = new AsyncLocalStorage();

app.mixins.push((service, path) => {
  const oldFind = service.find;
  service.find = function (params) {
    if (session.getStore()) {
      return oldFind.call(this, { session, ...params });
    }
    return new Promise((resolve) => {
      session.run(new Map(), () => {
        // session.getStore().set('time', new Date().getTime());
        return oldFind.call(this, { session, ...params ).then(resolve);
      });
    });
  };

// do the same for the rest of the methods
});

@jnardone
Copy link
Contributor

We do this in our application as well though we have to set up the initial storage outside of feathers (hooked into the transports for REST and socket.io) to maintain the same context throughout a single request. We used the async-local-storage library (https://github.com/vicanso/async-local-storage) to wrap the async hooks logic.

Would love to have something like this in feathers -- there are some things that really should have per-request visibility across calls without having to explicitly pass in params especially when that sometimes triggers additional logic (thinking about params.user specifically). Definitely helpful for auditing and logging contexts.

@avimar
Copy link

avimar commented Dec 16, 2020

I... don't particularly understand this async storage stuff.

I manually added a "trace uuid" in a global hook / middleware, and then have been making sure on each internal service invocation to pass that trace ID along, so that I can correlate all the queries afterwards. It's a bit verbose, but I got it to work on the server. This definitely has a bunch of boilerplate that I'm afraid of getting wrong, so having something that handles this automatically would be very cool.

(I didn't decide or yet what would be a single transaction from the client side.)

@DaddyWarbucks
Copy link
Member Author

DaddyWarbucks commented Dec 16, 2020

I have been thinking about this for a few years now and have come up with some other vanilla feathers solutions as well. For example.

// Extend classes or add mixin with a function that returns a service-like object that picks off
// params you want to pass along and automatically pass them
service.withSession = context => {
  return {
    find(params) {
      return service.find({ session: context.params.session, ...params })  
   }
  ...other methods
  }
}

// can be used like this in a hook
app.service("albums").withSession(context).find({ ... })

And another option is something like this

const sessionHook = context => {
  context.params.session = { ... }
  
  context.sessionService = (serviceName) => {
    find(params) {
      return context.app.service(serviceName).find({ session: context.params.session, ...params })  
   }
  ...other methods
  }
  
  return context;
}

// and can be used in a hook like this
context.sessionService('albums').find({ ... })

@DaddyWarbucks
Copy link
Member Author

DaddyWarbucks commented Aug 11, 2021

A quick update on my latest iteration of this and also including a ContextLoader from feathersjs-ecosystem/batch-loader#18 and an example for @1valdis.

// app.hooks.js

const { AsyncLocalStorage } = require('async_hooks');
const asyncLocalStorage = new AsyncLocalStorage();
const { ContextLoader } = require('./loaders');

const initializeSession = async (context, next) => {
  if (context.params.session) {
    // console.log('session manually passed', context.path);
    return next();
  }

  const currentSession = asyncLocalStorage.getStore();

  if (currentSession) {
    // console.log('continuing session', context.path);
    context.params.session = currentSession;
    return next();
  }

  return asyncLocalStorage.run(new Map(), async () => {
    // console.log('starting session', context.path);
    context.params.session = asyncLocalStorage.getStore();
    return next();
  });
};

const initializeLoader = (context, next) => {
  if (context.params.loader) {
    context.loader = context.params.loader;
    return next();
  }

  const currentLoader = context.params.session.get('loader');

  if (currentLoader) {
    context.params.loader = currentLoader;
    context.loader = currentLoader;
    return next();
  }

  const loader = new ContextLoader(context);
  context.params.session.set('loader', loader);
  context.loader = loader;

  return next();
};

module.exports = [initializeSession, initializeLoader];

This seems to be working well so far. I tried using a POJO instead of a Map for the session store to keep things straight forward. But, I was afraid of reassignment and/or destructuring. There is a large service area here for me to accidentally loose reference to the "true" state. For example,

// some hook later
context.params.session = { ...context.params.session, active: true };
// This is bad because we have reassigned (not mutated) params.session so it is
// a totally different object from asyncLocalStorage.getStore()

I think a better solution is to use a Map by convention. Sure you can still accidentally reassign params.session, but you are unlikely to do so because of the get and set API of a Map. Same example as above,

// some hook later
context.params.session.set('active': true);
// All good...no reassignment and asyncLocalStorage.getStore() is updated.

You may notice in the initializeLoader (checkout link above for more info), that I assign both context.loader = loader and context.params.loader = loader. This is not necessary, but a little pattern I am trying out to make using loaders really "first class". So the loader is always passed around via params/session, but it is also assigned to context to give it more prominence. Meh...we'll see how it goes.

// I don't love this because its on `params`. It feels less important than app.service
const result = await context.app.service('users').get(1);
const result2 = await context.params.loader.service('users').load(1);

// `context.loader.service` seems more naturally compared with `context.app.service`
const result = await context.app.service('users').get(1);
const result2 = await context.loader.service('users').load(1);

@J3m5
Copy link

J3m5 commented Aug 11, 2021

Sure you can still accidentally reassign params.session, but you are unlikely to do so because of the get and set API of a Map.

A solution could be to use Object.defineProperty and setting writable to false to prevent reassignment:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty

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

No branches or pull requests

6 participants