Delayed exception on undefined Callback in rest-adapter.js

[jbcpollak]

I have Loopback code that uses the loopback-connector-remote and does this:

Model.findById(function(err, model) {
    // my code
});

When I execute this code, I get:

.../node_modules/strong-remoting/lib/rest-adapter.js:134
          callback.apply(invocation, args);
                  ^

TypeError: Cannot read property 'apply' of undefined
    at .../node_modules/strong-remoting/lib/rest-adapter.js:134:16
    at execStack (.../node_modules/strong-remoting/lib/remote-objects.js:488:7)
    at RemoteObjects.execHooks (.../node_modules/strong-remoting/lib/remote-objects.js:492:10)
    at HttpInvocation.<anonymous> (.../node_modules/strong-remoting/lib/rest-adapter.js:132:15)
    at HttpInvocation.transformResponse (.../node_modules/strong-remoting/lib/http-invocation.js:337:12)
    at Request._callback (.../node_modules/strong-remoting/lib/http-invocation.js:261:10)
    at Request.self.callback (.../node_modules/request/request.js:199:22)
    at emitTwo (events.js:100:13)
    at Request.emit (events.js:185:7)
    at Request.<anonymous> (.../node_modules/request/request.js:1036:10)

Debugging into rest-adapter, it seems the callback variable is either lost or being cleared somehow. In the following screenshots you can see that callback is still valid until the call to invocation.invoke. When that runs, and its callback is executed, the callback variable has become undefined:

I've tried stepping through the code, but its pretty convoluted so I'm not really sure what is happening.

[jbcpollak]

I tracked the cause of this down - it was caused by an improper call on an unrelated (but also remote) model which was not causing an error. It took 3 days to hunt down.

As I said in the original post, somewhere in my project I had code like this:

Model.findById(function(err, model) {
    // my code
});

which was throwing the exception above.

Elsewhere in my code, I had:

ModelB.relation(null);

relation was defined as a belongsTo relationship.

calling ModelB.relation(null) did not result in an error, but I don't think it is a valid thing to do in loopback. When I removed the invalid call, the unrelated findById() started working.

Both Model and ModelB were remote objects, but with two different datasources pointing at different servers.

[jbcpollak]

I'm having the same problem when trying to call a custom remote method. In ModelB, (as described above) I have:

    ModelB.prototype.customMethod = function(param, done) {
                // do something
        done(null, {datetime: new Date()});
    };

    ModelB.remoteMethod(
        'customMethod',
        {
            isStatic: false,
            accepts: {arg: 'param', type: 'array', http: {source: 'body'}},
            returns: {arg: 'result', type: 'string'},
            http: {path: '/customMethod', verb: 'post'}
        }
    );

On my remote server, which is using loopback-connector-remote I call:

modelBInstance.customMethod(myParam);

The call works as expected.

Later I run modelBInstance.save() and I get the stack trace shown above.

If I comment out the call to the customMethod, everything works fine.

[jbcpollak]

ok, I've finally got it - this problem happens when the developer forgets that the call they are making is asynchronous, and then executes another async call at the same time. For example, I haven't 100% verified, but I suspect this would cause the same problem:

model.save()  // lets assume this takes some amount of time
modelB.save()

I would think strong-remoting should be able to detect this situation and at least warn the developer they screwed up.

Also, this makes me wonder - does this mean you can only make one async call at a time through this library? That seems like a potential performance issue.

[jbcpollak]

Now I understand the problem completely. The error I'm getting is that the callback is undefined - and this is actually true, but its the callback of the FIRST call, not the second call that is undefined, and the error isn't thrown until the second call is made, which is really confusing.

I can think of two solutions:

• Throw an exception immediately if the callback is null - in rest-adapter.js the callback is explicitly set to undefined - this might be a good place for better error handling.
• Just check in rest-adapter.js if the callback is undefined before calling it.

[richardpringle]

@jbcpollak, we have an improvement on error handling coming sometime soon (within the coming weeks I believe, but don't quote me on it).

I'll label this issue as a bug, but we should keep an eye out for the new error handler though. I'll comment in here when I have more details.

[richardpringle]

I actually think that the PR is right here #296

[bajtos]

@jbcpollak is this issue still relevant? Could you please provide us with a full code sample that we can run on our machine, e.g. using loopback-sandbox per our bug-reporting instructions?

[bajtos]

I am closing this issue since it's not reproducible. Feel free to reopen if you can provide us with instructions for reproducing it per my comment above.

[jbcpollak]

Thanks @bajtos, I think that's fine. I haven't had a chance to test the merged error handling code but I am guessing it covers the problem.