Re: [nodejs] Node calling back wrong object instance

23 views
Skip to first unread message

Ryan Schmidt

unread,
Jul 23, 2016, 10:26:44 PM7/23/16
to nod...@googlegroups.com

On Jul 23, 2016, at 9:13 AM, Phil Heltewig wrote:

> Hi all,
>
> I have a weird issue where I use two instances of the same object and call functions with callbacks from within these. At one stage one of the functions called from object 2 is calling back object 1 instead.
>
> Background
> I am trying to build a control flow library that allows you to specify a sequence of functions to be called and to branch based on conditions.
>
> The concept is that you pass the library a flow [a,b,c], which then executes functions a, b and c, passing the output values between them by calling back to the FlowProcessor, which then calls the next function in line, etc.
>
> Issue
> It all worked very well, until I added branching. Think this pseudocode: [a,b, {IF value=1 THEN [a,a,a] ELSE [b,b,b]}, c], encapsulating the IF in an object. Suppose value is 1, then the total flow should now be this: [a,b,[a,a,a], c]. The second [a,a,a] flow is executed by instantiating a new FlowProcessor object, which should then run to the end and return the result into the first flow.
>
> The weird thing happens next: After calling a in the second FlowProcessor, a calls back into the FIRST FlowProcessor object. My question is why?
>
> Code
> var FlowProcessor = function (id, flow, data, finalCallback) {
> this.id = id;
> this.input = data;
> this.flow = flow;
> this.counter = 0 - 1;
> this.flowLength = this.flow.length;
> console.log(this.id, 'New FlowProcessor instance');
>
> this.nextFlow = function (data) {
> this.counter++;
> if (this.counter < this.flow.length) {
> switch (typeof (this.flow[this.counter])) {
> case 'object':
> this.parseObject(data, this.flow[this.counter], function (result) {
> console.log('Object Parser called back');
> this.nextFlow(result);

Are you sure "this" refers to the correct object here? It's being used in an anonymous function passed as a callback parameter, which means it will be called "later", by which time "this" might refer to the global object instead of your class instance. To guard against that, at the beginning of nextFlow, make a reference to the correct "this":

var self = this;

Then in the remainder of nextFlow, use "self" anywhere you would have used "this".

This may not actually have anything to do with your problem, because it looks like parseObject immediately calls the callback. However, if you later changed parseObject to defer calling the callback until later, for example with setTimeout, you would notice that the "this" reference is wrong, therefore it's a good idea to always follow this pattern when callbacks are involved.


> });
> break;
> default:
> console.log(this.id, 'is calling function');
> this.flow[this.counter](this.id, data, function (result) {
> console.log(this.id, 'has been called back with result', result);
> this.nextFlow(result);

Same for this "this".

> });
>
> }
> } else {
> finalCallback(data);
> }
> };
>
> this.parseObject = function (data, obj, callback) {
> switch (obj.type) {
> case 'if':
> if (eval(obj.condition)) {
> console.log('Starting SubFlow (2)');
> var o = new FlowProcessor(2, obj.flowTrue, data, function (result) {
> console.log('Received Callback from SubFlow');
> callback(result);
> });
> o.startFlow();
> } else {
> var o = new FlowProcessor(2, obj.flowFalse, data, function (result) {
> callback(result);
> });
> o.startFlow();
> }
> break;
> default:
> callback(data);
> }
> };
>
> this.startFlow = function () {
> this.nextFlow(this.input);
> }
>
> return this;
> }
>
> var fp = FlowProcessor(1, [a, b, {type: 'if', condition: 'data<200', flowTrue: [a,a,a], flowFalse: [b,b,b]}, c], 1, function (result) {

Shouldn't this be "new FlowProcessor" instead of "FlowProcessor"? If you don't use "new", "this" within FlowProcessor refers to the global object, not a new instance of your class. You can add code to your constructor function to detect this incorrect usage and either throw an error or work around the incorrect usage and return a new instance anyway. For example, to do the latter, the first line in the FlowProcessor function could be:

if (!(this instanceof FlowProcessor)) return new FlowProcessor(id, flow, data, finalCallback);


> console.log(result);
> });
> fp.startFlow();
>
>
> function a(id, a, callback) {
> console.log(id, 'executing a');
> callback(a + 10);
> }
>
> function b(id, a, callback) {
> console.log(id, 'executing b');
> callback(a + 20);
> }
>
> function c(id, a, callback) {
> console.log(id, 'executing c');
> callback(a + 30);
> }
>
> Output:(the first number in the line shows the FlowProcessor ID). The issue is highlighted in red.
> 1 'New FlowProcessor instance'
> 1 'is calling function'
> 1 'executing a'
> 1 'has been called back with result' 11
> 1 'is calling function'
> 1 'executing b'
> 1 'has been called back with result' 31
> Starting SubFlow (2)
> 2 'New FlowProcessor instance'
> 2 'is calling function'
> 2 'executing a' //Flow 2 has called function a and executes it, then calls callback
> 1 'has been called back with result' 41 // Flow 1 receives the callback??
> 1 'is calling function'
> 1 'executing c'
> 1 'has been called back with result' 71
> 71
>
> What am I doing wrong?
>
> Thanks a lot,
> Phil


Reply all
Reply to author
Forward
0 new messages