Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fakeAsync in node throws TypeError: Cannot read property 'length' of undefined #760

Closed
tbosch opened this issue Apr 26, 2017 · 5 comments · Fixed by #762
Closed

fakeAsync in node throws TypeError: Cannot read property 'length' of undefined #760

tbosch opened this issue Apr 26, 2017 · 5 comments · Fixed by #762

Comments

@tbosch
Copy link

tbosch commented Apr 26, 2017

Node 6.9.5.

Stack:

TypeError: Cannot read property 'length' of undefined
    at onwriteDrain (_stream_writable.js:396:12)
    at afterWrite (_stream_writable.js:386:5)
    at ZoneDelegate.invokeTask (/Users/tbosch/projects/angular/node_modules/zone.js/dist/zone-node.js:399:31)
    at ProxyZoneSpec.onInvokeTask (/Users/tbosch/projects/angular/node_modules/zone.js/dist/proxy.js:103:39)
    at ZoneDelegate.invokeTask (/Users/tbosch/projects/angular/node_modules/zone.js/dist/zone-node.js:398:36)
    at Zone.runTask (/Users/tbosch/projects/angular/node_modules/zone.js/dist/zone-node.js:165:47)
    at ZoneTask.ZoneTask.cancelFn.invoke (/Users/tbosch/projects/angular/node_modules/zone.js/dist/zone-node.js:466:38)
    at FakeAsyncTestZoneSpec.flushMicrotasks (/Users/tbosch/projects/angular/node_modules/zone.js/dist/fake-async-test.js:204:17)
    at flushMicrotasks (../../../../../../packages/core/testing/src/fake_async.ts:135:27)

This happens after the test is done and fakeAsync flushes the microtasks.
The code in _stream_writable.js is from node, and contains the following:

process.nextTick(afterWrite, stream, state, finished, cb);
...
function afterWrite(stream, state, finished, cb) { ...}

The error above occurs because the call to process.nextTick does not forward the extra arguments to the afterWrite callback when it happens.

Workaround / patch in zone-node.js:

function patchMicroTask(obj, funcName, metaCreator) {
    var setNative = null;
    function scheduleTask(task) {
        var data = task.data;
        data.args[data.callbackIndex] = function () {
            task.invoke.apply(this, arguments);
        };
        // old: setNative.apply(data.target, data.args);
        // new:
        setNative.apply(data.target, [].slice.call(data.args, 0, data.callbackIndex+1));
        return task;
    }
    setNative = patchMethod(obj, funcName, function (delegate) { return function (self, args) {
        var meta = metaCreator(self, args);
        if (meta.callbackIndex >= 0 && typeof args[meta.callbackIndex] === 'function') {
            // new
            var cbArgs = [].slice.call(args, meta.callbackIndex+1);
            cbArgs.unshift(null);
            var task = Zone.current.scheduleMicroTask(meta.name, Function.bind.apply(args[meta.callbackIndex], cbArgs), meta, scheduleTask);
            // old: var task = Zone.current.scheduleMicroTask(meta.name, args[meta.callbackIndex], meta, scheduleTask);
            return task;
        }
        else {
            // cause an error by calling it directly.
            return delegate.apply(self, args);
        }
    }; });
}

I.e. bind the arguments early to the callback, so that fakeAsync does not have to pass them anymore.

Note that this is probably a design bug as:

  • the last argument to Zone.current.scheduleMicroTask contains method specific knowledge (in this case that the remaining argument have to be passed to the callback)
  • fakeAsync is never calling this callback.
@mhevery
Copy link
Contributor

mhevery commented Apr 26, 2017

The issue is that the FakeAsync ZoneSpec should emulate the semantics (just as it emulates the semantics of setTimeout) of the process.nextTick to correctly pass the additional arguments to it.

The fix is to have FakeAsync know to append the extra arguments when process.nextTick is flushed.

@JiaLiPassion
Copy link
Collaborator

I will try to fix it.

JiaLiPassion added a commit to JiaLiPassion/zone.js that referenced this issue Apr 27, 2017
JiaLiPassion added a commit to JiaLiPassion/zone.js that referenced this issue Apr 27, 2017
@JiaLiPassion
Copy link
Collaborator

@tbosch , I have fixed it in #762, could you please use the attached dist.zip to test it is ok or not?
I also added test case look like below

it('should be able to schedule microTask with additional arguments', () => {
               const process = global['process'];
               const nextTick = process && process['nextTick'];
               if (!nextTick) {
                 return;
               }
               fakeAsyncTestZone.run(() => {
                 let tickRun = false;
                 let cbArgRun = false;
                 nextTick(
                     (strArg: string, cbArg: Function) => {
                       tickRun = true;
                       expect(strArg).toEqual('stringArg');
                       cbArg();
                     },
                     'stringArg',
                     () => {
                       cbArgRun = true;
                     });

                 expect(tickRun).toEqual(false);

                 testZoneSpec.flushMicrotasks();
                 expect(tickRun).toEqual(true);
                 expect(cbArgRun).toEqual(true);
               });

             });

I think it should work!

dist.zip

@tbosch
Copy link
Author

tbosch commented Apr 27, 2017

@JiaLiPassion yes, this is working now, thanks!

@JiaLiPassion
Copy link
Collaborator

@tbosch , ok, got it! thank you for confirm.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants