search

moll / node-mitm

Intercept and mock outgoing Node.js network TCP connections and HTTP requests for testing. Intercepts and gives you a Net.Socket, Http.IncomingMessage and Http.ServerResponse to test and respond with. Super useful when testing code that hits remote servers.
Other
641 stars 48 forks source link

node 8 compatibility #46

Closed marcghorayeb closed 7 years ago

marcghorayeb commented 7 years ago

Hello,

We've been testing Mitm to mock some API calls when in a test environment and it works great under node 4.

I've been trying to test the module under node 8 and it seems that there are some failing tests. Furthermore, when running our project's test, we receive an uncaught exception:

_http_client.js:597
    const asyncId = socket._handle ? socket._handle.getAsyncId() : null;
                                                    ^

TypeError: socket._handle.getAsyncId is not a function
    at responseKeepAlive (_http_client.js:597:53)
    at IncomingMessage.responseOnEnd (_http_client.js:610:5)
    at emitNone (events.js:110:20)
    at IncomingMessage.emit (events.js:207:7)
    at endReadableNT (_stream_readable.js:1059:12)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickDomainCallback (internal/process/next_tick.js:218:9)

Is this module node 8 compatible ? Any idea what might be failing ?

Thanks for your help!

Best,

moll commented 7 years ago

While I don't use Node v8 myself yet, I do expect Mitm.js to work under it. The tests as they are now, do pass under Node v8. Checking out _http_client of Node v8 hints it's got something to do with keep-alive — an option the tests don't include. You're using keep-alive, right?

moll commented 7 years ago

I tracked this down, added a failing test and added the getAsyncId function Node v8 expects to exist. It's up as Mitm.js vs 1.3.3. ;) Let me know if it works out.

marcghorayeb commented 7 years ago

The tests seem to pass better, thanks a lot for the fix and the quick reply 👍