Loopback: Injecting Remote Context via Options

Created on 1 Jul 2015  ·  108Comments  ·  Source: strongloop/loopback

UPDATE 2016-12-22 We ended up implementing a different solution as described in https://github.com/strongloop/loopback/pull/3023.

Tasks

Original description for posterity

The example below allows you to inject the remote context object (from strong remoting) into all methods that accept an options argument.

function inject(ctx, next) {
  var options = hasOptions(ctx.method.accepts) && (ctx.args.options || {});
  if(options) {
    options.remoteCtx = ctx;
    ctx.args.options = options;
  }
  next();
}

app.remotes().before('*.*', inject);

app.remotes().before('*.prototype.*', function(ctx, instance, next) {
  inject(ctx, next);
});

// unfortunately this requires us to add the options object
// to the remote method definition
app.remotes().methods().forEach(function(method) {
  if(!hasOptions(method.accepts)) {
    method.accepts.push({
      arg: 'options',
      type: 'object',
      injectCtx: true
    });
  }
});

function hasOptions(accepts) {
  for (var i = 0; i < accepts.length; i++) {
    var argDesc = accepts[i];
    if (argDesc.arg === 'options' && argDesc.injectCtx) {
      return true;
    }
  }
}

Why? So you can use the remote context in remote methods, operation hooks, connector implementation, etc.

MyModel.observe('before save', function(ctx, next) {
  console.log(ctx.options.remoteCtx.accessToken.userId); // current user
});

This approach is specifically designed to allow you to do what is possible with loopback.getCurrentContext() but without the dependency on cls. The injection approach is much simpler and should be quite a bit faster since cls adds some significant overhead.

@bajtos @fabien @raymondfeng

feature major
Source
picture ritch
👍4

Most helpful comment

@chris-hydra

Our early apps used current context, we fell prey to CLS buggyness and did some rewriting to use a solution similar to what @ritch proposed in the description of this issue. We only did this 1x as we found it just caused us too much hassle trying to pass around the context all the time.

In the end what we settled on was to never use operation hooks which to my knowledge are the only place you can't just get at the context as you please. This was actually a double win for us as operation hooks we find tend to introduce side effects throughout your codebase that can be easy to forget about. (we use remote hooks instead)

So heres how we essentially share our state throughout a request lifecycle.

Set state on the request.

There are various ways you can access the request early enough to set desired state.
This sort of thing could be via middleware in server.js for example:

app.use((req, res, next) => {
  req.user.name = 'bob'
  next()
})

Though you can set state in components, mixins and boot scripts. Anywhere you can hook into the request lifecycle.

A boot script or component example

module.exports = function (app) {
  app.remotes().before('**', (ctx, cb) => {
    req.user.name = 'bob'
    cb()
  })
}

A Mixin example

module.exports = function (MyModel) {
  MyModel.beforeRemote('**', (ctx, instance, cb) => {
    req.user.name = 'bob'
    cb()
  })
}

Access state in a remote hook

MyModel.beforeRemote('create', (ctx, instance, cb) => {
  // ctx.req.user.name
  cb()
})

Access state in a remote method

MyModel.myMethod = (ctx, cb) => {
  // ctx.req.user.name
  cb()
}

MyModel.remoteMethod('myMethod', {
  accepts: [
    {arg: 'ctx', type: 'object', http: {source: 'ctx'}}
  ],
  returns: {root: true},
  http: {path: '/my-method', verb: 'get'}
})

I realise that this likely won't accommodate everyones use cases but for us, this has worked out great. We have some pretty decent size loopback apps now and have absolutely zero need for getCurrentContext. Hopefully this helps someone out.

picture digitalsadhu on 27 Sep 2016
👍17

All 108 comments

@ritch - the initial chunk of code sits in server/server.js correct?

pulkitsinghal picture pulkitsinghal on 2 Jul 2015

A boot script works fine... The goal is to add this to core

ritch picture ritch on 2 Jul 2015

The mechanism of passing the context object via the options argument looks ok, it's consistent with what we already do for transactions.

However, I dislike the fact that we want to expose the full remoting context to all model methods.

IMO, model methods should be as much transport (remoting-context) agnostic as possible. In my mind, here is the basic promise of LoopBack and strong-remoting: write your model methods once, then invoke them using whatever transport is most appropriate (REST RPC, JSON-RPC, WebSocket, MQTT, etc.). Once users start coupling their method implementation with remoting context details, it will be difficult to switch to a different transport. For example, I believe the websocket transport does not have the concept of req and res objects as provided by express.

We already had a similar discussion when implementing loopback.context() middleware, see https://github.com/strongloop/loopback/pull/337#issuecomment-59007194 and the following comments. The agreement was to implement a flag (disabled by default) which turns on this new behavior.

I am proposing to do the same here, for example:

// usage - code-first
app.enableRemotingContextInjection();

// usage via loopback-boot
// server/config.json
{
  port: 3000,
  enableRemotingContextInjection: true,
  // etc.
}

// implementation - provided by LoopBack
app.enableRemotingContextInjection = function() {
  app.remotes().before('*.*', inject);

  app.remotes().before('*.prototype.*', function(ctx, instance, next) {
    inject(ctx, next);
  });
  // etc.
};

Thoughts?

bajtos picture bajtos on 2 Jul 2015

I think we should leave users the choice, either this or use the cls approach. Sometimes you need access to the context, possibly even outside of the methods covered above.

fabien picture fabien on 9 Jul 2015

@ritch how would one set the accessToken or a user object?

eg. When I create a middleware after loopback.token() to set things up, how would I get access to ctx.options.remoteCtx to set things?

digitalsadhu picture digitalsadhu on 21 Jul 2015

nvm, I understand that ctx.options.remoteCtx is an object with req and res etc.

digitalsadhu picture digitalsadhu on 21 Jul 2015

I'm finding that using the above sample code does not inject the remoteCtx when calling a models methods directly.

Eg. I have a contractor model that hasMany contracts. The Contract.before('access', function () { }) hook is breaking when I try to access a contractor (ctx.options is undefined) but works fine when accessing the contract directly

Example:
In contractor.js

Contractor.afterRemote('**', function (ctx, next) {
  var Contract = Contractor.app.models.Contract
  Contract.find(function (contracts) {
    console.log(contracts)
  })
})

Blows up with a 500 error when accessing contractor via explorer with the message:
TypeError: Cannot read property 'req' of undefined which comes from the Contract.before('access') when trying to access ctx.options.remoteCtx.req which works fine when accessing Contract directly

digitalsadhu picture digitalsadhu on 21 Jul 2015

Just to summarize the error I'm seeing...

Anytime I access a related model, whether it's using includes in the url or just running a MyRelatedModel.find, the before access hooks in the myRelatedModel instance context.options does not contain remoteCtx.

digitalsadhu picture digitalsadhu on 24 Jul 2015

Anytime I access a related model, whether it's using includes in the url or just running a MyRelatedModel.find, the before access hooks in the myRelatedModel instance context.options does not contain remoteCtx.

This is a know issue of remoting hooks, see https://github.com/strongloop/loopback/issues/737. If you are accessing your Contract model via Contractor model, then you need to configure context-injecting hook for Contractor too.

bajtos picture bajtos on 5 Aug 2015

You need to pass the options.remoteCtx yourself in the example you mentioned above.

Contractor.afterRemote('**', function (ctx, next) {
  var Contract = Contractor.app.models.Contract
  var filter = {};
  var options = {remoteCtx: ctx};
  // the example above was also missing the `err` argument
  Contract.find(filter, options, function (err, contracts) {
    console.log(contracts);
  });
});

This highlights a downside of this approach: you must pass the remoteCtx around yourself. The advantage is you get to control when the remoteCtx is set. It is also much less magic (and error prone) than using the cls approach.

ritch picture ritch on 6 Aug 2015
👍1

This approach breaks User.login for us. The 3rd argument here: https://github.com/strongloop/loopback/blob/master/common/models/user.js#L169 is now the options arg (containing the remoteCtx object), which results in an error here: https://github.com/strongloop/loopback/blob/master/common/models/user.js#L228

ryedin picture ryedin on 6 Aug 2015

Thanks @ritch most helpful info. I think i'm almost there. Small problem though:

Via explorer: GET /contracts

Contract.observe('access', function (ctx, next) {
  //ctx has no access to remoting context
  next();
});

I believe this is just the same issue you describe a fix for above except that I'm not sure (since I'm not directly calling Contract.find) how I can inject the remote context via the options object you describe above. My understanding is that if I could directly call:

Contract.find(filter, { remoteCtx: ctx }, function (err, contracts) {  
})

then in: 

Contract.observe('access', function (ctx, next) {
  //ctx.options.remoteCtx will be set
});
digitalsadhu picture digitalsadhu on 15 Sep 2015

@digitalsadhu you need to inject the remoting context into options argument via a remoting hook. See the code in the issue description.

function inject(ctx, next) {
  var options = hasOptions(ctx.method.accepts) && (ctx.args.options || {});
  if(options) {
    options.remoteCtx = ctx;
    ctx.args.options = options;
  }
  next();
}

app.remotes().before('*.*', inject);

app.remotes().before('*.prototype.*', function(ctx, instance, next) {
  inject(ctx, next);
});

This code will inject remoting context to all models and all methods. If you prefer to inject the context only for Contract methods, then you can change the hook spec:

app.remotes().before('Contract.*', inject);

app.remotes().before('Contract.prototype.*', function(ctx, instance, next) {
  inject(ctx, next);
});
bajtos picture bajtos on 15 Sep 2015

Thanks @bajtos I am doing that in a context.js boot script. Thats working fine in most cases. There are a couple places where its not working:

Contract.observe('access') doesn't get ctx.options.remoteCtx where as Contract.observe('after save') does.

I should also say, I've logged out the injection and it does happen before the access hook is called and all looks well, it just never makes it onto the access hook ctx object

digitalsadhu picture digitalsadhu on 15 Sep 2015

This works:

Contract.observe('after save', function createHistory(ctx, next) {
    var History = Contract.app.models.History;
    var contract = ctx.instance;

    History.create({
      contractId: contract.id,
      createdBy: ctx.options.remoteCtx.req.user.name,
      snapshot: JSON.stringify(contract)
    }, next);
  });

This doesn't work:

Contract.observe('access', function limitContractStaff(ctx, next) {
  var user = ctx.options.remoteCtx.req.user;
  //ctx.options === {}
  if (user.group === roles.MANAGER)
    ctx.query.where = merge(ctx.query.where || {}, { createdBy: user.name });

  next();
});
digitalsadhu picture digitalsadhu on 15 Sep 2015

NVM, pretty sure its just an overly complicated model with side effects I wasn't aware of. Sorry for the spam ;)

digitalsadhu picture digitalsadhu on 15 Sep 2015

@digitalsadhu OK :) OTOH, it is possible that your model method doesn't have options argument and therefore the remoting hook does not inject remoteCtx.

bajtos picture bajtos on 15 Sep 2015

Nope. I've tracked down the last of my issues and now have a working solution. Every case was just me not explicitly passing {remoteCtx:ctx} to model methods such as find or findById etc. once I had tracked down all of those everything worked.

digitalsadhu picture digitalsadhu on 15 Sep 2015

@bajtos @ritch

FWIW The reason I've jumped through hoops to get rid of getCurrentContext with this approach is that I've repeatedly run into issues with getCurrentContext returning null with various connectors or platforms.
Eg. windows with mssql connector

The latest I've discovered (and should file a bug) is that if you use the memory connector and give a file to add persistence. The use of async.queue to save changes to the json file seems to be the culprit (I did some investigation and removed the use of async.queue and the issue went away).

I have a feeling you guys are already pretty aware of these types of issues and I suspect its mostly related to cls right?

Anyway, my comment being, I think it be really great to see a simpler approach such as the one provided in this issue used.

Thanks for all the help!

digitalsadhu picture digitalsadhu on 15 Sep 2015

The use of async.queue to save changes to the json file seems to be the culprit (I did some investigation and removed the use of async.queue and the issue went away).

Thanks for digging in this far...

I have a feeling you guys are already pretty aware of these types of issues and I suspect its mostly related to cls right?

Yes - cls is not a dependency I am comfortable with. The only reason we ever considered it was the simplicity in using getCurrentContext()... Your experience with it not reliably returning the context is what I have feared for a while. I would recommend moving away from getCurrentContext() and using my prescribed method. You have to be diligent about passing the remoteCtx around, which as you've seen is a bit tricky... but this is much easier than trying to figure out if getCurrentContext is broken or not every time you see it returning null.

ritch picture ritch on 15 Sep 2015

Tip:
For put requests to update a model eg. PUT model/1
I found I needed to modify my injection script like so:

app.remotes().before('*.prototype.*', function(ctx, instance, next) {
  if (typeof instance === 'function') {
    next = instance
  }
  inject(ctx, next);
});

As for some reason instance is the callback function and next is some other context like object. (Perhaps instance and next are reversed?)

A bit odd but seems to do the trick.

digitalsadhu picture digitalsadhu on 15 Sep 2015

@ritch @bajtos
I've found some significant performance issues with this approach.

The problem:
Significantly increased wait times (experiencing 20 second TTFB) with multiple simultaneous requests. You don't really notice anything if you just perform 1 request after another (with eg. curl.)

This function is the culprit:

app.remotes().methods().forEach(function(method) {
  if(!hasOptions(method.accepts)) {
    method.accepts.push({
      arg: 'options',
      type: 'object',
      injectCtx: true
    });
  }
});

As soon as that code is commented out, problem goes away.

Any ideas why this might be? Is there anyway I can work around this? (Other than reducing simultaneous queries to the server)

digitalsadhu picture digitalsadhu on 16 Sep 2015

@ritch @bajtos Ok figured it out. You can't just inject the entire httpcontext. It's just not performant. I am now injecting my user object directly and all is well again.

function inject(ctx, next) {
  var options = hasOptions(ctx.method.accepts) && (ctx.args.options || {});
  if(options) {
    options.user = ctx.req.user;
    ctx.args.options = options;
  }
  next();
}

@ritch One further thing to note is that the use of the hasOptions method in your implementation above results in some remote methods getting 2 options args because if the remote method already had an options object arg that did not have the injectCtx property on it then it is treated as having no options object at all. A new one then gets added and so accepts ends up like: 

accepts: [
  {arg: 'options', type: 'object'},
  {arg: 'options', type: 'object', injectCtx: true}
]

Not a huge deal as in practice this only happens for the change-stream methods

digitalsadhu picture digitalsadhu on 17 Sep 2015
👍2

I have some ideas for solving the performance issues and keeping a clean separation between remote hooks and impls.

ritch picture ritch on 17 Sep 2015

Will link the PR when I have something concrete. Thanks for all the feedback.

ritch picture ritch on 17 Sep 2015

@ritch N.P. Awesome will look forward to PR.

digitalsadhu picture digitalsadhu on 17 Sep 2015

I am experiencing the same issue but the current solution doesn't work for me. I have the logged user in the options as suggested in a remote method , but my remote method is calling updateAttribute and this calls before save and in before save i don't have the logged user in the options object

ezraroi picture ezraroi on 11 Oct 2015

Also looks like it is breaking the login method, this is the error I am getting after adding the suggested code:

2015-10-11T08:05:47.393Z - error: uncaughtException: fn is not a function 
{ date: 'Sun Oct 11 2015 11:05:47 GMT+0300 (Jerusalem Daylight Time)',
  process: 
   { pid: 9604,
     uid: null,
     gid: null,
     cwd: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager',
     execPath: 'C:\\Program Files\\nodejs\\node.exe',
     version: 'v4.1.2',
     argv: 
      [ 'C:\\Program Files\\nodejs\\node.exe',
        'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\server\\server.js' ],
     memoryUsage: { rss: 122175488, heapTotal: 93701632, heapUsed: 63284664 } },
  os: { loadavg: [ 0, 0, 0 ], uptime: 264889.8970633 },
  trace: 
   [ { column: 9,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback\\common\\models\\user.js',
       function: 'tokenHandler',
       line: 228,
       method: null,
       native: false },
     { column: 13,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\relation-definition.js',
       function: null,
       line: 1814,
       method: null,
       native: false },
     { column: 19,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\dao.js',
       function: null,
       line: 320,
       method: null,
       native: false },
     { column: 49,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js',
       function: 'doNotify',
       line: 93,
       method: null,
       native: false },
     { column: 49,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js',
       function: 'doNotify',
       line: 93,
       method: null,
       native: false },
     { column: 49,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js',
       function: 'doNotify',
       line: 93,
       method: null,
       native: false },
     { column: 49,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js',
       function: 'doNotify',
       line: 93,
       method: null,
       native: false },
     { column: 5,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js',
       function: 'Function.ObserverMixin._notifyBaseObservers',
       line: 116,
       method: 'ObserverMixin._notifyBaseObservers',
       native: false },
     { column: 8,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js',
       function: 'Function.ObserverMixin.notifyObserversOf',
       line: 91,
       method: 'ObserverMixin.notifyObserversOf',
       native: false },
     { column: 15,
       file: 'c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js',
       function: 'Function.ObserverMixin._notifyBaseObservers',
       line: 114,
       method: 'ObserverMixin._notifyBaseObservers',
       native: false } ],
  stack: 
   [ 'TypeError: fn is not a function',
     '    at tokenHandler (c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback\\common\\models\\user.js:228:9)',
     '    at c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\relation-definition.js:1814:13',
     '    at c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\dao.js:320:19',
     '    at doNotify (c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js:93:49)',
     '    at doNotify (c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js:93:49)',
     '    at doNotify (c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js:93:49)',
     '    at doNotify (c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js:93:49)',
     '    at Function.ObserverMixin._notifyBaseObservers (c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js:116:5)',
     '    at Function.ObserverMixin.notifyObserversOf (c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js:91:8)',
     '    at Function.ObserverMixin._notifyBaseObservers (c:\\Users\\rezra3\\Documents\\GitHub\\shutdownManager\\node_modules\\loopback-datasource-juggler\\lib\\observer.js:114:15)' ] }

I am using In-Memory connector

ezraroi picture ezraroi on 11 Oct 2015

@ezraroi are you passing your options through to updateAttributes? See Ritchs comment above about needing to pass the options object along yourself.

digitalsadhu picture digitalsadhu on 27 Oct 2015

Hello, I refactored my code as you suggested with remote ctx, but then I can 't login to the system because of:

TypeError: fn is not a function on
node_modules\loopback\common\modelsuser.js:261:9

Seems that code modifies all remote methods including standard ones.

rimvydas-urbonas picture rimvydas-urbonas on 25 Nov 2015

Yeah, what do we have to do to not break existing code with this hack? Does it end with injecting it into login? And why is it so hard to keep the ctx?

Morriz picture Morriz on 8 Feb 2016

any updates on a more performant and solid/recommended way from LB?

wprater picture wprater on 8 Feb 2016

@ritch

  1. this work around fails for methods *.prototype.updateAttritubes, for example, PUT rest api calls to /Model/{id}, because the next in this case is the second parameter and not the third!
  2. to get the userid, shouldn't it be ctx.options.remoteCtx.req.accessToken.userId instead of ctx.options.remoteCtx.accessToken.user
eggerdo picture eggerdo on 12 Feb 2016

Is this issue affecting dynamic role resolvers? We are using resolvers for our ACL and wonder if the context passed to a resolver is "save" to use, or is it actually suffering the same problem as getCurrentContext() described here ?

To illustrate an example of a resolver, we do something like this:

// /server/boot/script.js
module.exports = function(app) {
  var Role = app.models.Role;
  Role.registerResolver('teamMember', function(role, context, cb) {
    var userId = context.accessToken.userId; // could this be affected by this issue?

    if (userId === 1) {
      cb(null, true);
    } else {
      cb(null, false);
    }
  }
}

Note that the resolver function is called before any hooks registered with app.remotes().before('*.*', inject) fire, so the injected context will not be present.

hotaru355 picture hotaru355 on 2 Mar 2016
👍2

Another issue with using the proposed work-around: there doesn't seem to be a way to have access to the remoteCtx within a custom validation function.

User.validate('name', customValidator, {message: 'Bad name'});

function customValidator(err) {
  // the validation function does not get the options passed in, so how do we get the injected context??
  var userId = loopback.getCurrentContext().get('accessToken').userId; // how do we replace this?
  if (userId === 1) err();
});
hotaru355 picture hotaru355 on 3 Mar 2016

be careful using this workaround. if you have multiple concurrent requests hitting a remote method that is using this, you will likely get cyclical loop in the SharedMethod.convertArg method.

the issue is the traverse(raw).forEach(...) which seems to get caught up on some async flow with the concurrent http reqs.

wasn't fun to track down.
image

you can work around it in two ways, both of which will skip this traverse, which is not needed on the HttpContext you're setting here.

1 . method.accepts.push can setup the config object to use http.source === 'context' and be sure to directly set the argument you created with the actual HttpContext. something like this (I renamed mine)

server.remotes().methods().forEach(method => {
  if (!hasOptions(method.accepts)) {
    method.accepts.push({
      arg: 'httpCtx',
      description: '**Do not implement in clients**.',
      type: Object,
      injectCtx: true,
      source: 'context'
    });
  }
});
  function inject(ctx, next) {
    const httpCtx = hasOptions(ctx.method.accepts) && ctx;
    if (httpCtx) ctx.args.httpCtx = httpCtx;
    next();
}

2 . use a custom object to inject as the options.

  function RemoteCtx(ctx) {
    Object.assign(this, ctx);
  }

then when you are setting the ctx in inject, be sure to create a new instance of this object.

httpCtx = new RemoteCtx(ctx);
options.remoteCtx = httpCtx;

hope this is helpful!

wprater picture wprater on 6 Mar 2016
👍1

Any progress on this?

I think the proposed workaround is pretty dirty (because related model methods have a different signature, methods like login/logout have to be excluded, ... )

kevinsschmidt picture kevinsschmidt on 23 Apr 2016
👍1

@ritch this doesn't seem to work with methods with relationships, for example model1.prototype.__create__model2 - I get an error of next is not a function in the server.js file.

Any thoughts there? Any other alternatives?

tomhallam picture tomhallam on 10 May 2016
👍1

The user.login would crash the app for the injected new argument:

prj\node_modules\loopback\common\models\user.js:262
            fn(defaultError);
            ^

TypeError: fn is not a function
  at prj\node_modules\loopback\common\models\user.js:262:13
  at prj\node_modules\loopback\common\models\user.js:313:9

Why not attach the remoteCtx as session to the context of the operator hooks directly?

Model.observe('access', function(context, next){
  var remoteCtx = context.session;
  if (remoteCtx) {...}
  next()
})
snowyu picture snowyu on 28 May 2016

One of the method where options is not available is isValid method of model

pkgulati picture pkgulati on 1 Jun 2016

Can I lobby to have the issues with getCurrentContext() and this suggestion/issue page etc referenced in the loopback docs (e.g., https://docs.strongloop.com/display/public/LB/Using+current+context)?

Could save others some serious headaches.

roberttaylortech picture roberttaylortech on 26 Jun 2016
👍4

Note: After implementing this all of my api calls -- both Rest and Angular bindings -- now require options param (neither work and the app hangs when options is not included!)

These both work:

//Angular Binding

MainUser.find({
    options:{},
    filter:{
        limit: params.count(),
        skip: params.page() * params.count()
    }

}, function(data){

    debugger;

})

//Rest URL
"/api/MainUsers?filter=%7B"limit":10,"skip":10%7D&options=%7B%7D"

This won't work:
//Rest URL
"/api/MainUsers?filter=%7B"limit":10,"skip":10%7D

Will look into it... I did change the options param to required:false... to no avail.

Thoughts??
roberttaylortech picture roberttaylortech on 26 Jun 2016

@souluniversal It should be Model.find(filter, options).

snowyu picture snowyu on 27 Jun 2016

I create a component with black and white list supports:
https://github.com/snowyu/loopback-component-remote-ctx.js

snowyu picture snowyu on 27 Jun 2016

btw: I create a component with black and white list supports.
https://github.com/snowyu/loopback-component-remote-ctx.js

snowyu picture snowyu on 28 Jun 2016

Sitting late at night and deploying our newly developed solution on customers staging server for tomorrow's presentation when suddenly this getCurrentContext()==null issue occurs. It's been working on our dev machines and our test server without hassle.

Feeling literally nauseous about this whole thing at the moment, with no time to rewrite the codebase, especially when recommended workaround breaks vital functionalities as login/logout etc.

For the moment I'm too exhausted to bring more constructive ideas to the table which leaves me more or less just whining, my apologies. But I really think this is an issue we need to address in a more comprehensive manner and bring a reliable - yet easy to use - solution.

denkan picture denkan on 14 Jul 2016
👍12

We also experienced a sudden entrance of the getCurrentContext() is null. The suggested solution does not sound very appealing. Attaching a custom object to every remote method is not something we like to do.

However, there is the option of passing the request and response to a custom method. You just add them to your accepted arguments. From there you will be able to get the user token etc.

For example:

AppUser.remoteMethod('upload', {
    http: {path: '/files', verb: 'post'},
    accepts: [
      {arg: 'req', type: 'object', http: {source: 'req'}},
      {arg: 'res', type: 'object', http: {source: 'res'}},
    ],
  })

Then you can define your custom method as
AppUser.upload = ( req, res, next) => { ..}

I don't know how loopback attaches the arguments to the remote method. But it seems to be consistent. One of the many _magical_ features loopback offers.

pbtcjasper picture pbtcjasper on 14 Jul 2016
👍2

A potential fix that I've found (if it's working fine on your dev machine) is to run npm shrinkwrap and install/deploy again with the npm-shrinkwrap.json present in the deployed bundle.

It seems that various subdependencies/relationships cause this issue, though I have no idea which combination does it.

tomhallam picture tomhallam on 14 Jul 2016
👍1

We also experienced this all of a sudden in the last few days. Nothing changed directly on our end. My guess is that a dependency of a dependency somewhere updated their version of the async module (which I know can cause this issue). I spent the best part of 2 days trying to rework things following the recommendation here. I was using the https://github.com/snowyu/loopback-component-remote-ctx.js component from @snowyu and whilst I can get that working, I can not bring myself to update our entire codebase of 40+ models and thousands of methods and operation hooks to try to manually pass the context through to every single function. Manually passing the context around like that brings a spaghetti nightmare and an unmaintainable (or readable) codebase.

We too have resorted to using npm shrinkwrap to lock things down to a known working state. Without doing this there is no way to guarantee that what works on one build will not fail on the next as the issue can be brought on simply by the order in which npm installs the dependency tree.

The proposed solution from this thread is not a viable alternative to cls IMHO.

mrfelton picture mrfelton on 14 Jul 2016
👍3

@mrfelton +1 for this solution not being viable for a larger codebase. Glad that the npm-shrinkwrap approach worked for you.

tomhallam picture tomhallam on 14 Jul 2016

+1 for this method being a poor solution.

I refactored all of my code and tests after this popped up randomly only to discover that it does not work on related methods like contractor.prototype.__create__contract.

It appears that the options arg is not being persisted for related models. You can check method.accepts during runtime and see that it does not have options even after verifying that it is pushed on in this code.

app.remotes().methods().forEach(function(method) {
    if(!hasOptions(method.accepts)) {
        method.accepts.push({
            arg: 'options',
            description: '**Do not implement in clients**',
            type: 'object',
            injectCtx: true
        }); 
    }   
});
zanemcca picture zanemcca on 15 Jul 2016
👍3

For anyone who is interested here is an updated version of the hack that takes into account all of the previously listed errors, except for the related methods error.

  /*  
   * Sets up options injection for passing accessToken and other properties around internally
   */
  var setupOptionsInjection = function() {
    function inject(ctx, next) {
      var options = hasOptions(ctx.method.accepts) && (ctx.args.options || {});
      if(options) {
        options.accessToken = ctx.req.accessToken;
        ctx.args.options = options;
      }   
      next();
    }   
    function hasOptions(accepts) {
      for (var i = 0; i < accepts.length; i++) {
        var argDesc = accepts[i];
        if (argDesc.arg === 'options' && argDesc.injectCtx) {
          return true;
        }   
      }   
    }   

    if(!process.env.GENERATING_SDK) {
      app.remotes().before('*.*', inject);

      app.remotes().before('*.prototype.*', function(ctx, instance, next) {
        if (typeof instance === 'function') {
          next = instance;
        }   
        inject(ctx, next);
      }); 

      var blacklist = ['login', 'logout', 'confirm', 'resetPassword'];

      // unfortunately this requires us to add the options object
      // to the remote method definition
      app.remotes().methods().forEach(function(method) {
        if(!hasOptions(method.accepts) && blacklist.indexOf(method.name) === -1) {
          method.accepts.push({
            arg: 'options',
            description: '**Do not implement in clients**',
            type: 'object',
            injectCtx: true
          }); 
        }   
      }); 
    }   
  };

This takes into account the bug pointed out by @wprater and a fix for it by @digitalsadhu.
It also adds a blacklist that will ignore certain functions such as login and logout.This fixes the issues noticed by @ryedin and others. The GENERATING_SDK env var can be set before running lb-ng to avoid having options in the angular sdk.

zanemcca picture zanemcca on 15 Jul 2016
👍5

@richardpringle is there any update on a fix for this issue?

zanemcca picture zanemcca on 15 Jul 2016
👍3

As @hotaru355 @zanemcca mention to "model.prototype.__create__relation" not work for @ritch 's solution. Because prototype.methods resolve by dynamically

https://github.com/strongloop/strong-remoting/blob/master/lib/shared-class.js#L130

My solution is walking around by acl.

module.exports = function(app) {

  var Role = app.models.Role;
  var merchantStaffRoles = ['owner', 'manager', 'cashier'];

  function checkOptions(method) {
    if(!hasOptions(method.accepts)) {
      method.accepts.push({
        arg: 'options',
        type: 'object',
        injectCtx: true
      });
    }
  }

  function hasOptions(accepts) {
    for (var i = 0; i < accepts.length; i++) {
      var argDesc = accepts[i];
      if (argDesc.arg === 'options' && argDesc.injectCtx) {
        return true;
      }
    }
  }

  function errorWithStatus(msg, status) {
    var error = new Error(msg);
    error.status = status;
    return error;
  }

  // Inspire by https://github.com/strongloop/loopback/issues/1495
  Role.registerResolver('merchantStaff', function (role, ctx, next) {
    if(!ctx.accessToken.userId) return process.nextTick(()=>next(errorWithStatus('Forbidden anonymouse user', 401), false));
    app.models.User.findById(ctx.accessToken.userId, function(err, user) {
      if (err || !user) {
        if(!user) err = errorWithStatus('Forbidden anonymouse user', 401);
        return next(err, false);
      }
      checkOptions(ctx.sharedMethod);
      var options = ctx.remotingContext.args.options||{};
      options.currentUser = user;
      ctx.remotingContext.args.options = options;
      next(err, merchantStaffRoles.indexOf(user.role)>-1);
    });
  });

  // Another poor solution, not work for model.prototype.__create__relation
  // https://github.com/snowyu/loopback-component-remote-ctx.js

};
guanbo picture guanbo on 19 Jul 2016
👍2

Are there any updates on this issue?

pongstr picture pongstr on 25 Jul 2016
👍6

Why is this still a problem? We're about to go live with our app which has been in development for months and now this happens... Damn, this is not good at ALL...

skyserpent picture skyserpent on 25 Jul 2016
👍13

In fact, I do not like this tricky. Why not to pass the current-context as an optional argument to the model's event directly.

snowyu picture snowyu on 26 Jul 2016

I agree that the each operation should have the remoting context when triggered by a user.

The scope of this change could be pretty big though and will have to be evaluated.

@raymondfeng @ritch @bajtos, what do you guys think?

richardpringle picture richardpringle on 27 Jul 2016

How can I access the remote context in a middle ware, it just have (req, res, next) ?

dongmai picture dongmai on 3 Aug 2016

I used monkey patching of strong remoting SharedMethod.prototype.invoke method to inject options, similar to callback. We can set req.contextOptions in middlewares, this gets passed to juggler and you can receive this in all observer hooks or user hooks.

        var injectedOptions = ctx.req.contextOptions
        formattedArgs.push(injectedOptions);

        // add in the required callback
        formattedArgs.push(callback);
pkgulati picture pkgulati on 10 Aug 2016

@souluniversal

Can I lobby to have the issues with getCurrentContext() and this suggestion/issue page etc referenced in the loopback docs (e.g., https://docs.strongloop.com/display/public/LB/Using+current+context)?

Could save others some serious headaches.

Good idea, done.

bajtos picture bajtos on 10 Aug 2016

~Since this issue is now referenced in the docs, I think it's good to reference yet another (completely different) alternative, even if not done (yet)~
~https://github.com/strongloop/loopback-context/pull/2~
~TL;DR; use the native-ish cls-hooked(which is good for Node 6 and future Node releases) instead of continuation-local-storage, and detect (unfortunately not at compile time) a very common source of bugs with cls which were successfully reproduced (maybe only a subset of the possible bugs, so be warned).~ PR closed

josieusa picture josieusa on 10 Aug 2016

FWIW: if you are writing a custom remote method and all you need is currentUserId, then the following approach may work well for you:

MyModel.customFunc = function(data, currentUserId, cb) {
  // ...
};

MyModel.remoteMethod('customFunc', {
  accepts: [
    // arguments supplied by the HTTP client
    { arg: 'data', type: 'object', required: true, http: { source: 'body' } },
    // arguments inferred from the request context
    { 
      arg: currentUserId, type: 'any', 
      // EDITED
      http: function(ctx) { return ctx.req.accessToken && ctx.req.accessToken.userId; }
      // doesn't work
      // http: function(req) { return req.accessToken && req.accessToken.userId; }
    },
  ],
  // ...
});
bajtos picture bajtos on 10 Aug 2016
👍6 ❤1

@dongmai

How can I access the remote context in a middle ware, it just have (req, res, next) ?

When using loopback-context#per-request, you can use req.loopbackContext to access the CLS container that will be returned by LoopBackContext.getCurrentContext().

bajtos picture bajtos on 10 Aug 2016

@dongmai

I think I misunderstood what you are asking. I don't think there is an easy way how to access remoting context from a middleware. Middleware typically stores data on the req object. Then you can create a remoting hook to process the data attached on the req object and make any changes to the remoting context as needed.

bajtos picture bajtos on 10 Aug 2016

Any updates on this issue?

I was also about to deploy to production when the context started to get lost. I have tried using the npm-shrinkwrap solution but it's not working for me.

Any particular version that I should stick to for continuation-local-storage?

Currently, my npm-shrinkwrap file contains the following cls version:

"continuation-local-storage": {
          "version": "3.1.7",
          "from": "continuation-local-storage@>=3.1.3 <4.0.0",
          "resolved": "https://registry.npmjs.org/continuation-local-storage/-/continuation-local-storage-3.1.7.tgz",
          "dependencies": {
            "async-listener": {
              "version": "0.6.3",
              "from": "async-listener@>=0.6.0 <0.7.0",
              "resolved": "https://registry.npmjs.org/async-listener/-/async-listener-0.6.3.tgz",
              "dependencies": {
                "shimmer": {
                  "version": "1.0.0",
                  "from": "[email protected]",
                  "resolved": "https://registry.npmjs.org/shimmer/-/shimmer-1.0.0.tgz"
                }
              }
            },

Are there any particular steps I should follow to make npm-shrinkwrap alleviate this problem? or are there any other potential workarounds? it's very furstrating because it seems that sometimes after re-deploying several times it ends up working.

Thanks!

Saganus picture Saganus on 18 Aug 2016

@bajtos I have tried your solution. the following is my code.

ip and userId is undefined

  Card.remoteMethod('preorder', {
    isStatic: false,
    accepts: [
      {
        arg: 'ip', type: 'any',
        http: function (req) {return req.ip}
      },
      {
        arg: 'userId', type: 'any',
        http: function (req) {return req.accessToken && req.accessToken.userId}
      }
    ],
    returns: [
      {arg: 'appId', type: 'string', description: '公众号名称'},
      {arg: 'timeStamp', type: 'string'},
      {arg: 'nonceStr', type: 'string'},
      {arg: 'package', type: 'string'},
      {arg: 'signType', type: 'string'},
      {arg: 'paySign', type: 'string'}
    ],
    http: {path: '/preorder', verb: 'post', errorStatus: 400}
  });

  Card.prototype.preorder = function (ip, userId, callback) {

    console.log(ip);
    console.log(userId);
}
JoeShi picture JoeShi on 18 Aug 2016

@JoeShi

Same thing happened to me when trying to return the accessToken. One thing I've noted in the past is that for some reason req doesn't seem to have attached the accessToken object. At least it's not always available. I'm not sure why.

I'm thinking on having to parse the query to get the access_token and then just query the DB to get the userId corresponding to that token and use that instead. Unfortunately it will add another query for each call.

Saganus picture Saganus on 18 Aug 2016

Disclaimer: I plan to do the same for options injection too soon. I'm not trying to hijack the original intent of this issue.

Here's an idea for those who are still sticking with CLS:

  1. If the following people can share their exact versions for nodejs, npm and the output file called npm-shrinkwrap.json after running npm shrinkwrap then I can look into setting up all of the permutations on Docker and publish a table of results around which ones work and which ones don't.

    1. @Saganus, @skyserpent , @pongstr , @snowyu, @mrfelton and @tomhallam ... I think that's everyone on this thread who's in bed with cls ... if I guessed wrong, my apologies.

  2. Any simplified code (based on your usecases) that you can share for quickly testing if CLS is working, would also be appreciated! I will volume-mount such code into all the docker containers and then maybe use apache-ab or something to test the server. If someone has unit tests, or functional test so that I don't have to build a test harness, even better!
  3. I understand that connectors are a big part of the puzzle so I hope to also test across mongo and memory. If you want me to test additional ones then you may have to help me out a bit by giving me a simple sandbox env that's ready to go. For example, I prepared ones for connectors that I'm familiar with:

    1. mongo - https://github.com/ShoppinPal/loopback-mongo-sandbox/blob/master/docker-compose.yml

    2. elasticsearch - https://github.com/strongloop-community/loopback-connector-elastic-search/blob/master/docker-compose-for-tests.yml

I'm volunteering for running a large-scale experiment to narrow down the issue of what works most easily, therefore, anything you can pitch in to the trial will help.

pulkitsinghal picture pulkitsinghal on 18 Aug 2016
👍1

Fwiw we have stopped using currentContext as well as operation hooks and
never looked back. Have yet to come across a situation where this has been
a problem for us. (We have a half dozen medium to large loopback apps in
production)

You can always get ahold of req inside custom remote methods and before and
after remote hooks and use that to pass state or get access to the user and
token.
On Thu, 18 Aug 2016 at 21:03, Pulkit Singhal [email protected]
wrote:

Here's an idea for those who are still sticking with CLS:

  1. If the following people can share their exact versions for nodejs, npm
    and the output file called npm-shrinkwrap.json after running npm
    shrinkwrap then I can look into setting up all of the permutations on
    Docker and publish a table of results around which ones work and which ones
    don't.
  2. @Saganus https://github.com/Saganus, @skyserpent
    https://github.com/skyserpent , @pongstr https://github.com/pongstr
    , @snowyu https://github.com/snowyu,

    1. Any simplified code (based on your usecases) that you can share

      for quickly testing if CLS is working, would also be appreciated! I will

      volume-mount such code into all the docker containers and then maybe use

      apache-ab or something to test the server. If someone has unit

      tests, or functional test so that I don't have to build a test harness,

      even better!

    2. I understand that connectors are a big part of the puzzle so I

      hope to also test across mongo and memory. If you want me to test

      additional ones then you may have to help me out a bit by giving me a

      simple sandbox env that's ready to go. For example, I prepared ones for

      connectors that I'm familiar with:

  3. mongo -
    https://github.com/ShoppinPal/loopback-mongo-sandbox/blob/master/docker-compose.yml
  4. elasticsearch -
    https://github.com/strongloop-community/loopback-connector-elastic-search/blob/master/docker-compose-for-tests.yml

I'm volunteering for running a large-scale experiment to narrow down the
issue of what works most easily, therefore, anything you can pitch in to
the trial will help.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/strongloop/loopback/issues/1495#issuecomment-240823473,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABH2CjIvCZRVQ3Jl5jD2a2IR2zilxWbqks5qhKxrgaJpZM4FQCkF
.

digitalsadhu picture digitalsadhu on 19 Aug 2016
🎉2 😄1

Hi @pulkitsinghal,

Thanks for offering to do this.

Regarding the code I'm not sure I'll be able to extract a small working example since in my case I've only experienced CLS issues when deploying to Heroku. I can however share my npm-shrinkwrap.json with the versions I'm using.

By the way, I was still having issues even after using npm-shrinkwrap, however I was not clearing the cache dir in Heroku so I guess that could've prevented it from working, since after I purged the cache dir and redeployed everything then things seem to started working ok (however not sure for how long or if we'll have the issue again or not).

I think I'm going to do as @digitalsadhu and remove all use of the currentContext and see if we can get rid of the issues.

One question for @digitalsadhu: you say you also got rid of operationHooks. Are they also causing context problems or were they removed for different reasons?

I ask because I also use operationsHooks and I'm not yet aware of any issues but I also am not looking very hard, however you just made me worry a bit :)

Thanks!

Btw, my shrinkwrap file is here:

http://pastebin.com/8vrbcnSU

Thanks for the help and hard work!

Saganus picture Saganus on 19 Aug 2016
👍1

No issues with operation hooks as such it's just that they are afaik the
only place where you can't get at the req without current context. In our
experience before and after remote hooks are enough.
On Fri, 19 Aug 2016 at 16:11, Akram Shehadi [email protected]
wrote:

Hi @pulkitsinghal https://github.com/pulkitsinghal,

Thanks for offering to do this.

Regarding the code I'm not sure I'll be able to extract a small working
example since in my case I've only experienced CLS issues when deploying to
Heroku. I can however share my npm-shrinkwrap.json with the versions I'm
using.

By the way, I was still having issues even after using npm-shrinkwrap,
however I was not clearing the cache dir in Heroku so I guess that could've
prevented it from working, since after I purged the cache dir and
redeployed everything then things seem to started working ok (however not
sure for how long or if we'll have the issue again or not).

I think I'm going to do as @digitalsadhu https://github.com/digitalsadhu
and remove all use of the currentContext and see if we can get rid of the
issues.

One question for @digitalsadhu https://github.com/digitalsadhu: you say
you also got rid of operationHooks. Are they also causing context problems
or were they removed for different reasons?

I ask because I also use operationsHooks and I'm not yet aware of any
issues but I also am not looking very hard, however you just made me worry
a bit :)

Thanks!

Btw, my shrinkwrap file is here:

http://pastebin.com/8vrbcnSU

Thanks for the help and hard work!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/strongloop/loopback/issues/1495#issuecomment-241028843,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABH2CvOCySjvD4aOO93Xv3PNyksvcTMaks5qhbl8gaJpZM4FQCkF
.

digitalsadhu picture digitalsadhu on 19 Aug 2016

Is there any plan or a fix branch for this, this is really frustrating two days before the launch.

blackvirus18 picture blackvirus18 on 31 Aug 2016

@pulkitsinghal

  1. I can't share any code besides what I published on my account
  2. Here is a test, follow README in order to setup it (beware that this uses cls-hooked instead of continuation-local-storage, and that you'll need to clone my forked repo and checkout the branch cls-hookedand install Node >= 4.
    https://github.com/josieusa/loopback-context/blob/bf874aa3376b3edc4a6ee64d93855e7420bc71c3/test/main.test.js#L115
  3. In the case of cls-hookedI don't know if it would be useful to test connectors. In fact, it uses async-hook which patches these functions: setTimeout, clearTimeout, setInterval, clearInterval, process.nextTick, promise.prototype.then, as you can see here.
    https://github.com/AndreasMadsen/async-hook/tree/9c2d1ee927233ec1ad31cbf286029c1c98bd9885/patches
    In the case of cls-hooked, I think that when it fails it's because they are not patched in time, before the connectors use these functions. That's why I wrote the test the way I did.
josieusa picture josieusa on 7 Sep 2016

@pulkitsinghal @Saganus @josieusa please open a new issue to discuss this, or perhaps use https://gitter.im/strongloop/loopback instead, so that we can keep this discussion focused on the original proposal, which is to allow an alternative way for injecting context into remote methods, one that does not rely on CLS.

bajtos picture bajtos on 7 Sep 2016
👍2

Hello everybody watching this issue, I have just submitted a patch proposing a solution:

https://github.com/strongloop/loopback/pull/2762

I am encouraging you to take a look and let us know what do you think about my proposal.

bajtos picture bajtos on 20 Sep 2016
👍4

Cons:

  1. The model observe could miss the options If some developer forget to add the options parameter on the remote method which do some CRUD operation.
  2. Not for all build-in remote methods yet.
  3. Need to notify all the component developers to modify their codes.
  4. The written remote method must meet the specification, or crash the server(maybe use the Promise only to solve it). eg,

``` js
Note.remoteMethod('greet', {
isStatic: true,
accepts: [
{ arg: 'msg', type: 'string', http: { source: 'query' } },
{ arg: 'options', type: 'object', http: loopback.optionsFromContext },
],
returns: { arg: 'greeting', type: String },
http: { verb: 'get'}
});

 Note.greet = function(msg, cb) {
   process.nextTick(function() {
     msg = msg || 'world';
     cb(null, 'Hello ' + msg); //crash here. 
   });
 };

```

snowyu picture snowyu on 21 Sep 2016

On Aug 19 @digitalsadhu said:

Fwiw we have stopped using currentContext as well as operation hooks and
never looked back. Have yet to come across a situation where this has been
a problem for us. (We have a half dozen medium to large loopback apps in
production)

@digitalsadhu, could you elaborate on the final form of your approach to replace currentContext?

I see about 6 variations on the solution discussed above, and as a SL noob I would appreciate a simple code sample, a "with CLS" and "no CLS" kind of thing... ; )

chris-hydra picture chris-hydra on 27 Sep 2016

Hello again,

I came to this page from: https://docs.strongloop.com/display/public/LB/Using+current+context, which links to this page with the following (emphasis mine):

Refer to loopback issue #1495 updates and an alternative solution.

As I mentioned in my effort to contact @digitalsadhu, I'm trying to sort through all of the "alternative solutions" mentioned above in this thread to understand what the fix will entail.

I see @ritch commented on Sep 17, 2015

Will link the PR when I have something concrete. Thanks for all the feedback.

And @bajtos commented 7 days ago

Hello everybody watching this issue, I have just submitted a patch proposing a solution:

2762

I know everyone's busy and this is OSS, but if the SL docs send me to this page for an alternative solution, which one is it?

As a front-end dev covering for our back-end dev (away on holidays) I'm really reluctant to jump in and start re-inventing yet another fix to this - is there a recommended solution from SL, and if so, where is it?

chris-hydra picture chris-hydra on 27 Sep 2016

@chris-hydra

Our early apps used current context, we fell prey to CLS buggyness and did some rewriting to use a solution similar to what @ritch proposed in the description of this issue. We only did this 1x as we found it just caused us too much hassle trying to pass around the context all the time.

In the end what we settled on was to never use operation hooks which to my knowledge are the only place you can't just get at the context as you please. This was actually a double win for us as operation hooks we find tend to introduce side effects throughout your codebase that can be easy to forget about. (we use remote hooks instead)

So heres how we essentially share our state throughout a request lifecycle.

Set state on the request.

There are various ways you can access the request early enough to set desired state.
This sort of thing could be via middleware in server.js for example:

app.use((req, res, next) => {
  req.user.name = 'bob'
  next()
})

Though you can set state in components, mixins and boot scripts. Anywhere you can hook into the request lifecycle.

A boot script or component example

module.exports = function (app) {
  app.remotes().before('**', (ctx, cb) => {
    req.user.name = 'bob'
    cb()
  })
}

A Mixin example

module.exports = function (MyModel) {
  MyModel.beforeRemote('**', (ctx, instance, cb) => {
    req.user.name = 'bob'
    cb()
  })
}

Access state in a remote hook

MyModel.beforeRemote('create', (ctx, instance, cb) => {
  // ctx.req.user.name
  cb()
})

Access state in a remote method

MyModel.myMethod = (ctx, cb) => {
  // ctx.req.user.name
  cb()
}

MyModel.remoteMethod('myMethod', {
  accepts: [
    {arg: 'ctx', type: 'object', http: {source: 'ctx'}}
  ],
  returns: {root: true},
  http: {path: '/my-method', verb: 'get'}
})

I realise that this likely won't accommodate everyones use cases but for us, this has worked out great. We have some pretty decent size loopback apps now and have absolutely zero need for getCurrentContext. Hopefully this helps someone out.

digitalsadhu picture digitalsadhu on 27 Sep 2016
👍17

@digitalsadhu, thank you for taking the time to write this up - I hope other folks will get some use out of this as well.

chris-hydra picture chris-hydra on 27 Sep 2016
👍2

@digitalsadhu, your approach seems to be natural and simple. How can it be used to access the state from the model's code, for example from the overrided CRUD method?

aorlic picture aorlic on 28 Sep 2016

@aorlic
We typically disable the CRUD endpoint in question and implement our own as a new remote method injecting ctx as needed.

eg. replacing find would look something like this

MyModel.disableRemoteMethod('find', true)

MyModel.customFind = (filter, ctx, cb) => {
  // ctx.req.someState is now available
  MyModel.find(filter, cb)
}

MyModel.remoteMethod('customFind', {
  accepts: [
    {arg: 'filter', type: 'object', http: {source: 'query'}}
    {arg: 'ctx', type: 'object', http: {source: 'ctx'}}
  ],
  returns: {root: true},
  http: {path: '/', verb: 'get'}
})

Please excuse any mistakes, code above is untested and mostly going from memory.

I especially like this approach as the behaviour of the ORM's find method stays the same, only the endpoint is overridden.

digitalsadhu picture digitalsadhu on 28 Sep 2016
👍3

@digitalsadhu, I get it, thank you. Seems like a valid workaround, as the external REST endpoints remain just the same.

However, I just cannot believe that a serious API Server does not permit to legally override a CRUD method (without this kind of hacking) and access the context from it.

What else a developer is expected to do in an API Server, if no to play around with CRUDs?

aorlic picture aorlic on 28 Sep 2016

@digitalsadhu, one more question for you. In case of "overriding" a findById method this way, what should go here?

http: {path: '/', verb: 'get'}

What does it mean returns: {root: true}?

I can't make it work for this case...

Thank you very much!

aorlic picture aorlic on 28 Sep 2016

@aorlic looking at the loopback source code, the definition for findById here:
https://github.com/strongloop/loopback/blob/master/lib/persisted-model.js#L733
looks like:

{
    description: 'Find a model instance by {{id}} from the data source.',
    accessType: 'READ',
    accepts: [
      { arg: 'id', type: 'any', description: 'Model id', required: true,
        http: { source: 'path' }},
      { arg: 'filter', type: 'object',
        description: 'Filter defining fields and include' },
    ],
    returns: { arg: 'data', type: typeName, root: true },
    http: { verb: 'get', path: '/:id' },
    rest: { after: convertNullToNotFoundError },
}

{root: true} just means that whatever the remote method returns is returned at the root of returned payload.

See the "argument descriptions" section of the remote methods docs here:
https://docs.strongloop.com/display/public/LB/Remote+methods

digitalsadhu picture digitalsadhu on 28 Sep 2016

For those looking for a workaround and details, here is a nice resource on the subject : http://blog.digitopia.com/tokens-sessions-users/

kartsims picture kartsims on 5 Oct 2016
👍2

I am struggling to understand, exactly how to utilize any of the above solutions with remote operations like loaded, and access.

We have middleware that extracts a JWT, and in the past has stored data from the jwt in the current context. Then in remote operations such as loaded, or access we do some work, depending on values in the jwt.

getCurrentContext is seemingly randomly returning null now and no amount of shrinkwrap, or reverting library versions seems to have resolved it.

fperks picture fperks on 5 Oct 2016

@fperks can you extract the jwt info and stick it on the req object in your middleware then use beforeRemote and afterRemote hooks instead of access and loaded operation hooks?

digitalsadhu picture digitalsadhu on 5 Oct 2016

@digitalsadhu i feel that would require a significant amount of time and effort given the size of our project. Not to mention the amount of testing and other requirements that we would need to go through again.

I am more looking for a simple and clean solution to this problem.

fperks picture fperks on 5 Oct 2016

@fperks Ah, fair enough.
I'm afraid I'm not sure such a solution exists. :(

digitalsadhu picture digitalsadhu on 5 Oct 2016

I've posted my workaround and some observations here: https://github.com/strongloop/loopback/issues/1676

fabien picture fabien on 21 Oct 2016

@digitalsadhu Thank you for your suggestion! This is the great solution, we never getCurrentContext and never got currentContext bug. It's really cool.

Just a small updating on step Access state in a remote method

MyModel.remoteMethod('myMethod', {
  accepts: [
    {arg: 'ctx', type: 'object', http: {source: 'req'}}
  ],
  returns: {root: true},
  http: {path: '/my-method', verb: 'get'}
})

http.source can be req or context is also OK. More info http://loopback.io/doc/en/lb2/Remote-methods.html#http-mapping-of-input-arguments

myvo picture myvo on 8 Nov 2016

hi all, there is already a ctx.options object referenced here for operation hooks that

enables hooks to access any options provided by the caller of the specific model method (operation)

also when you look at the ctx context available for exemple at role resolving stage, there is no such ctx.optionsobject, but there is a ctx.remoteContext.options. Anything you put in this later object is then already available in ctx.options in remote method hooks.

Couldn't we not just 'bridge' the ctx.remoteContext.options object with the operation hooks ctx.optionsobject to make the ctx.options behavior consistent all along the workflow ?

This way, one could choose to pass any options in this object and retrieve them in any related piece of code through the app.

One one my regular concern is indeed passing information from custom role resolver to any possible methods and related hooks so that i don't need to do again the role resolving (db requests consuming) in the models when needed for advanced CRUD access control.

It seems @guanbo is trying similar things here by going through the options req argument

ebarault picture ebarault on 10 Nov 2016

Keep this fresh, we are using the "Loopback Component Group Access"-module to enable group access to our resources. Of course this module requires the getCurrentContext which forced us to downgrade multiple modules to make things work. Such a basic thing shouldn't become an complicated puzzle of module versions to make it work.

Undrium picture Undrium on 17 Nov 2016

@Undrium please check the solution I managed to set up here, gathering a lot of individual contributions
https://github.com/strongloop/loopback/issues/1676#issuecomment-260088138

ebarault picture ebarault on 17 Nov 2016

Hi everyone,
following my comment here on Aug 10, I closed that PR and deleted that branch (in favor of the newer strongloop/loopback-context/pull/11 which works for me and is only waiting for review).
EDIT Please use #2728 for comments about it, and not this, thank you

josieusa picture josieusa on 22 Dec 2016

Hello, the patch allowing remote methods to receive remoting context via options argument has been landed to both master (3.x) and 2.x.

Remaining tasks:

Please note that I am still intending to land (and release) the solution proposed by @josieusa in strongloop/loopback-context#11 too, as it offers an important alternative to our "official" approach.

bajtos picture bajtos on 22 Dec 2016

Ouch, I pressed "comment" button too soon. The pull request for 2.x is waiting for CI right now (see #3048).

bajtos picture bajtos on 22 Dec 2016

@bajtos

as it offers an important alternative to our "official" approach

which do you consider as the target approach ?

ebarault picture ebarault on 22 Dec 2016

@bajtos When this update is published, is there a possibility it will break 2.x projects using the workaround that already uses options?

tvdstaaij picture tvdstaaij on 22 Dec 2016

@ebarault

which do you consider as the target approach ?

Propagating context via "options" argument is our recommended and supported approach. While it has downsides (e.g you have to manually add options argument to all your methods and be disciplined in passing them around), it is also the only solution that is 100% reliable (meaning a change in a dependency like a database driver won't break it).

I don't know how reliable the new loopback-context version based on cls-hooked will be in practice. AFAIK, "continuation local storage" in Node.js is still relying on subtle tricks like pre-loading cls-hooked as the first module, monkey-patching certain Node.js API, etc. As a result, there may be edge cases where CLS-based solutions don't work and thus we cannot recommend a CLS-based context propagation as a universal tool for all LoopBack apps.

@tvdstaaij

When this update is published, is there a possibility it will break 2.x projects using the workaround that already uses options?

That's a very good question, indeed. It really depends on how well your workaround handles existing options arguments. AFAICT, the code you pointed to uses injectCtxflag to recognize options arguments already added. Because LoopBack itself does not have any concept of injectCtx, the built-in options argument do not include this flag. As a result, I think you will end up with two options arguments and the application will break, as the second options argument will be passed in as a value for callback function argument.

The main reason for back-porting this new context propagation to 2.x is to allow the recently added access-token invalidation skip the access token of the current user making the request, which I consider as an important bug to be fixed (see #3034).

I am happy to discuss how to minimise the impact of this back-ported change on existing LoopBack 2.x applications. Please move this discussion to #3048.

bajtos picture bajtos on 22 Dec 2016

📢 I have released this new feature in both 2.x (2.37.0) and 3.x (3.2.0) 📢

bajtos picture bajtos on 9 Jan 2017

The documentation at http://loopback.io/doc/en/lb3/Using-current-context.html was updated too (please allow few minutes for the changes to propagate to the website).

I am calling this issue finally done 🎉

bajtos picture bajtos on 11 Jan 2017
🎉7

This is great work, thanks!

One question though, any recommended path to upgrade from 2.29.1 to 2.37.0?

I mean, are there any specific warnings or issues I should be aware of? Or is it enough to just change the package.json loopback version and then installing it?

I know I have to clean up my code to stop using the previous context code, but is it as simple as removing it before the update, installing the new version, and then using the new features as described in the docs?

Thanks again for solving this!

Saganus picture Saganus on 11 Jan 2017

@Saganus Yes, upgrading should be as easy as you described:

  1. Update LoopBack version
  2. Set injectOptionsFromRemoteContext in your model JSON files
  3. Rework your methods to use the new context propagation API

However, there may be community-maintained modules that may have issues with the new computed "options" argument, see e.g. https://github.com/mean-expert-official/loopback-sdk-builder/issues/310. I recommend to run your test suite (including client tests) after enabling injectOptionsFromRemoteContext in step 2, just to make sure all layers of your app correctly support computed arguments.

bajtos picture bajtos on 12 Jan 2017
🎉1

On node 8.9.1 with the MongoDB connector, getCurrentContext returns null if it is not set outside of the call to a model like:

req.app.models.User.findById(req.accessToken.userId, function (err: Error, user: any) {
    var lbc = loopbackContext.getCurrentContext({ bind: true });
    if (lbc) {
        lbc.set('currentUser', user);
    }
    next();
});

However, it does work when you move it outside:

var lbc = loopbackContext.getCurrentContext({ bind: true });
req.app.models.User.findById(req.accessToken.userId, function (err: Error, user: any) {
    if (lbc) {
        lbc.set('currentUser', user);
    }
    next();
});

You also must use the bind option or it will return null as well. The documentation on loopback-context will have you pulling your hair out because it will not work as written.

matt-landers picture matt-landers on 25 Nov 2017
👍1

I have cross-posted the comment above to https://github.com/strongloop/loopback-context/issues/21.

I am going to lock down this issue to collaborators only, please open new issues if there is anything else to discuss.

bajtos picture bajtos on 14 Dec 2017
Was this page helpful?
0 / 5 - 0 ratings

Related issues

imasif picture imasif  ·  3Comments
nmklong picture nmklong  ·  3Comments
AnupDeshpande23 picture AnupDeshpande23  ·  4Comments
bajtos picture bajtos  ·  3Comments
cajoy picture cajoy  ·  4Comments