New way to deal with async and error handling

[Alexsey]

During adaptation of this library to my needs I have come out with the interface that I think much easier to use and less verbose for async testing and additionally, it allows to test errors:

const createMocks = require('path/to/custom/wrapper')
const {req, res, err, result} = createMocks({
  method: 'GET',
  url: '/route/under/test'
  query: {x: 1}
})
routeUnderTest(req, res, err)

const [error, send] = await result

send.should.eql({x: 1})

Where custom wrapper looks like

'use strict'

const httpMocks = require('node-mocks-http')

httpMocks.createResponse({eventEmitter: require('events').EventEmitter})

async function createMocks (reqParams, resParams) {
    let errResolver
    const error = new Promise(res => errResolver = res)

    const {req, res} = httpMocks.createMocks(reqParams, resParams)

    let dataResolver
    const data = new Promise(res => dataResolver = res)
    res.on('end', () => dataResolver(res._getData()))

    const result = await Promise.race([Promise.all([error, null]), Promise.all([null, data])])
    return {req, res, err: errResolver, result}
}

module.exports = createMocks

So it solves problems:

1. Sync response._getData()
2. Dealing with event emitter that is verbose and error-prone (there is a bunch of issues on there)
3. Provides an interface to test errors

I would like to discuss here:

1. Would you like to merge this or similar interface in the core
2. How would you like it to be

If we would come to some solution here I would request some time to make a PR on that

[eugef]

Hi @Alexsey, could you elaborate on problem 1 and 2? What exactly you are trying to improve.

Regarding 3rd problem: I am following this approach to test errors:

it('throws an exception', () => {
        const req = httpMocks.createRequest();
        const res = httpMocks.createResponse();
        const next = sinon.spy();

        let error;
        try {
            someMiddleware()(req, res, next);
        } catch (e) {
            error = e;
        }

        expect(error).to.be.instanceOf(Error);
        expect(error.message).to.equal('Error text')

        expect(next).not.be.called;
    });

What is your opinion about it?

[Alexsey]

Right now there are two interfaces to test response - _getData and .on('end', ...) for sync and async responses. Problem with the _getData one is that it is just silently not working for async and problem with .on('end', ...) is that it requires you set up your response object with error emitter than subscribe to event and then getting your flow back to the test that is very verbose. And this complexity produces issues https://github.com/howardabrams/node-mocks-http/issues/149 https://github.com/howardabrams/node-mocks-http/issues/113 and a few I can't find right away

While I am proposing a single interface that is implementing in one line

Same about your approach for test error - I was doing the same for the first time I have started to use the library but it's really verbose as for something that should be repeated hundreds of times

[Alexsey]

BTW I'm not sour that my current implementation is not relying on express-async-errors so maybe it would need some more tweaks even for this basic scenario to work in general. But here I would like to discuss an interface - like would you like to have something like this and if so how should we export variables and error, how should we name them etc

[eugef]

I never run into troubles with testing async middlewares because i was using async/await in my tests. As this feature is officials supported in Node 8 - can it be a solution in your case?

[Alexsey]

My example is based on async/await so yes, I'm already using them :) It's not about current approach is not working - it's about the interface. It's:

1. Double - separate for sync and separate for async
2. Heavy - async case requires you to provide event emitter to the request object options and add an event listener for 'end' event
3. Verbose - all above adds a test boilerplate that increases the size of test up to twice, requires nesting and continuation passing (for 'end' event handler)
4. Same for the error handling

vs

single line with await

[Alexsey]

Actually we can even remove that single line and make it

const createMocks = require('path/to/custom/wrapper')
const {req, res, err, send} = createMocks({
  method: 'GET',
  url: '/route/under/test'
  query: {x: 1}
})
routeUnderTest(req, res, err)

await send.should.eventually.eql({x: 1})

and for errors

const createMocks = require('path/to/custom/wrapper')
const {req, res, err, error} = createMocks({
  method: 'GET',
  url: '/route/under/test'
  query: {x: 1}
})
routeUnderTest(req, res, err)

await error.should.eventually.eql(Error('Some custom error'))

if createMocks would provide an object where error and send are already separate promises

Or it could be

const createMocks = require('path/to/custom/wrapper')
const {req, res, err, result} = createMocks({
  method: 'GET',
  url: '/route/under/test'
  query: {x: 1}
})
routeUnderTest(req, res, err)

await result.should.eventually.eql({x: 1})

and for errors

const createMocks = require('path/to/custom/wrapper')
const {req, res, err, result} = createMocks({
  method: 'GET',
  url: '/route/under/test'
  query: {x: 1}
})
routeUnderTest(req, res, err)

await result.should.be.rejectedWith(Error('Some custom error'))

if result would resolves to send and rejectes to error

[Alexsey]

Another approach may be to go from createMocks interface and completely remove the code duplication:

const createMocks = require('path/to/custom/wrapper')

const [error, send] = await testRouteWith(routeUnderTest, {
  method: 'GET',
  url: '/route/under/test'
  query: {x: 1}
})

send.should.eql({x: 1})

Where testRouteWith first argument is the middleware under test, and second and third are the same as first and second in createMocks

[github-actions[bot]]

Stale issue message