Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

is it possible to ignore mongodb requests? #30

Open
joelmukuthu opened this issue Dec 4, 2017 · 5 comments
Open

is it possible to ignore mongodb requests? #30

joelmukuthu opened this issue Dec 4, 2017 · 5 comments

Comments

@joelmukuthu
Copy link
Member

i have a case where my handler is making some mongodb calls and a request to a remote server. in my tests i'd like to mock out only the upstream server requests but it seems that unexpected-mitm also considers the mongodb requests as upstream traffic. i'm not really sure why this is happening and perhaps i'm just missing something obvious but here's a minimal example reproducing the problem:

// test.js
const express = require('express');
const request = require('request-promise');
const httpception = require('httpception');
const { MongoClient, Server } = require('mongodb');

const sendUpstreamRequest = async () =>
  request({
    method: 'POST',
    url: 'http://foo.bar/'
  });

const connectToMongoDb = async () => {
  const uri = 'mongodb://127.0.0.1:27017';
  const client = new MongoClient(new Server(uri));
  await client.connect(uri);
};

const expect = require('unexpected')
  .clone()
  .use(require('unexpected-express'))
  .addAssertion(
    '<object|string> to respond with <object|number>',
    (expect, req, res) => {
      const app = async (req, res, next) => {
        try {
          await connectToMongoDb();
          await sendUpstreamRequest();
          res.send('OK');
        } catch (e) {
          next(e);
        }
      };

      return expect(app, 'to yield exchange', {
        request: req,
        response: res
      });
    }
  );

it('foos', async () => {
  httpception({
    request: 'POST http://foo.bar/',
    response: 200
  });
  await expect('/', 'to respond with', 200);
});

i also have mocha installed, and when i run the test with ./node_modules/.bin/mocha --exit test.js, it times out with this output:

  1) foos
  2) "after each" hook for "foos"

  0 passing (2s)
  2 failing

  1) foos:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
  

  2) "after each" hook for "foos":
     
expected
function () {
  const innerPromise = expect.promise(function (resolve, reject) {
    // This promise will be resolved once afterEach calls us.
    resolveFromAfterEach = resolve;
    // ... lines removed ...
  });

  return innerPromise;
}
with http mocked out allowing modification [ { request: 'POST http://foo.bar/', response: 200 } ] not to error

// missing:
// POST /
// Host: foo.bar
//
// HTTP/1.1 200 OK

     at Array.forEach (native)
  From previous event:
      at node_modules/httpception/lib/httpception.js:100:35
      at httpception (node_modules/httpception/lib/httpception.js:86:37)
      at Context.it (test.js:42:3)
      set UNEXPECTED_FULL_TRACE=true to see the full stack trace

i'm using httpception but it's the same problem if using unexpected-mitm. also, the test passes if i comment out the await connectToMongoDb(); call. is there a way to get unexpected-mitm to ignore the mongodb requests?

@papandreou
Copy link
Member

Hmm, funny, I've never really needed this, http really is everywhere :)

It'd be fairly easy to listen to mitm's connect event and call socket.bypass() for the connect attempts that shouldn't be intercepted:

mitm.on('connect', function (socket, opts) {
  if (opts.port === 27017) socket.bypass();
});

... but that's not currently exposed. The hardest problem is to come up with a good API :)

@alexjeffburke and I recently talked about assigning a response function to handle every request to a given service, and whatever syntax we'd end up with for that could probably also be used to express that certain connections should be left alone.

Any ideas?

@alexjeffburke
Copy link
Member

Hey, thanks for the report and sorry I’ve not had much chance to think about this.

-mitm and by extension httpception does it’s nagic at the lower http module layer in node, so any http traffic will fall under its purview. I didn’t think mongo used HTTP as a transport for its BSON data but maybe i’m wrong or it’s changed.

As @papandreou mentioned, we have discussed alternative mock specification syntaxes, some of which might allow you to do different things based on request categories - if we did it by hostname for example - but there’s a lot to work out there. I’ll ponder it some.

That being said, I wonder a little about the calling code. I don’t know what it looks like, but assuming it’s something like a handler then it might make sense to construct it with thebupstream clients, at which point you’d leave the upstream component you want to test but swap in stubbed mongodb.

Note, the reason for the above suggestion is needing a pass through feels to me a little like the test might currently be doing too much. Be interesting to hear some more of the background too.

@joelmukuthu
Copy link
Member Author

Hey, thanks for replying :)

Regarding the structure of the calling code, I was refactoring some huge block of code so I wanted to have some "integration" tests that I would ensure are passing as I nuke/refactor the old code. I found a way around this so I'm not currently blocked on it.

About that API, we could add mitm options as a second argument to httpception:

httpception({
  request: 'POST http://foo.bar/',
  response: 200
}, {
  bypass: { 
    host: 'foo', // or maybe a regex 
    port: 1234 
  }
  // or maybe: bypass: /^mongodb:\/\//  
})

but that doesn't really work with unexpected-mitm. so maybe add support for mitm options in the http exchange object:

httpception({
  request: 'POST http://foo.bar/',
  response: 200,
  options: { // mitmOptions?
    bypass: { 
      host: 'foo', 
      port: 1234 
    }
  }
})

the mitm options could also go into the request object but someone might interpreted that as "request options".

@papandreou
Copy link
Member

An easy fix could be to ignore traffic to non-HTTP ports unless there are specific mocks for the given host:port combo. It would devalue the "with no http traffic taking place" use case, though.

It'd be nicer to make it explicit, but it's hard to shove more settings into the with http mocked out config object.

Maybe we should try to expose a few more knobs in unexpected-mitm and then play with possible APIs in httpception?

httpception({
  request: 'POST http://foo.bar/',
  response: 200,
  options: { // mitmOptions?
    bypass: { 
      host: 'foo', 
      port: 1234 
    }
 }
})

Hmm, I don't think it makes sense to configure it as a property of a single exchange like that. Remember that the generalized form looks like:

httpception([
  { request: ..., response: ... },
  { request: ..., response: ... },
  ...
]);

I think I'd rather expose these features via separate httpception.something(...) calls, such as:

// Let connections to *:27017 through without hijacking:
httpception.ignorePort(27017);

// Register a response function that responds to all requests to the given endpoint(s), outside of the timeline:
httpception.service(
  'http://myservice.com:1234/*',
  (req, res) => {
    let myMessage; 
    if (req.url === '/thatOneExpectedThing') {
      myMessage = '<h1>to be expected</h1>';
    } else {
      myMessage = '<h1>how very unexpected</h1>';
    }
    res.writeHead(200, { 'Content-Type': 'text/plain' });
    res.end(myMessage);
  }
);

// Mock a single request outside of the timeline.
// The assertion will pass whether or not it was exercised,
// but we might want to expose that info somehow so you
// can choose to assert that it happened if it matters for the given test:
httpception.optional(
  { request: ..., response: ... }
);

@joelmukuthu
Copy link
Member Author

@papandreou sorry for the late reply (I did a mental reply and forgot to comment here). I agree with everything you said :) would be nice to have this but it's not an absolute priority for me atm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants