From 7f40e52a33ec3407f97f4735f69c785ea154f857 Mon Sep 17 00:00:00 2001 From: Giancarlo Anemone Date: Thu, 30 Aug 2018 10:55:18 -0400 Subject: [PATCH] Use end event rather than overwriting ctx.res.end (#43) https://github.com/fusionjs/fusion-plugin-http-handler/pull/43 --- src/__tests__/express-send.node.js | 51 ++++++++++++++++++++++++++++++ src/server.js | 41 ++++++++++++++++-------- 2 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 src/__tests__/express-send.node.js diff --git a/src/__tests__/express-send.node.js b/src/__tests__/express-send.node.js new file mode 100644 index 0000000..5ecd89f --- /dev/null +++ b/src/__tests__/express-send.node.js @@ -0,0 +1,51 @@ +/** Copyright (c) 2018 Uber Technologies, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import test from 'tape-cup'; +import App from 'fusion-core'; +import express from 'express'; +import {HttpHandlerToken} from '../tokens.js'; +import HttpHandlerPlugin from '../server.js'; +import {startServer} from '../test-util.js'; + +test('http handler with express using send', async t => { + const app = new App('test', () => 'test'); + app.register(HttpHandlerPlugin); + const expressApp = express(); + expressApp.get('/express', (req, res) => { + res.send('OK'); + }); + app.register(HttpHandlerToken, expressApp); + app.middleware((ctx, next) => { + if (ctx.url === '/express') { + t.equal(ctx.res.statusCode, 200, 'express route sets status code'); + } else { + t.equal(ctx.res.statusCode, 404, 'non express routes default to 404'); + } + // $FlowFixMe + ctx.req.secure = false; + ctx.body = 'hit fallthrough'; + return next(); + }); + + const {server, request} = await startServer(app.callback()); + + t.equal(await request('/express'), 'OK', 'express routes can send responses'); + t.equal( + await request('/fallthrough'), + 'hit fallthrough', + 'express routes can delegate back to koa' + ); + t.equal( + await request('/fallthrough'), + 'hit fallthrough', + 'express routes can delegate back to koa' + ); + server.close(); + t.end(); +}); diff --git a/src/server.js b/src/server.js index ff7d7c4..dcefe9a 100644 --- a/src/server.js +++ b/src/server.js @@ -9,10 +9,8 @@ /* eslint-env node */ import {createPlugin} from 'fusion-core'; -import {HttpHandlerToken} from './tokens.js'; - import type {FusionPlugin} from 'fusion-core'; - +import {HttpHandlerToken} from './tokens.js'; import type {DepsType, ServiceType} from './types.js'; const plugin = @@ -29,23 +27,38 @@ const plugin = return next(); } return new Promise((resolve, reject) => { - const oldEnd = ctx.res.end.bind(ctx.res); - // $FlowFixMe - ctx.res.end = (data, encoding, cb) => { + const {req, res} = ctx; + // Default http response object behavior defaults res.statusCode to 200. Koa sets it to 404. + // This allows for http servers to use `end()` or express to use `send()` without specifying a 200 status code + const prevStatusCode = ctx.res.statusCode; + ctx.res.statusCode = 200; + const listener = () => { ctx.respond = false; - return next() - .then(resolve) - .then(() => { - oldEnd(data, encoding, cb); - }); + return done(); }; - handler(ctx.req, ctx.res, () => { + res.on('end', listener); + res.on('finish', listener); + + handler(req, res, () => { + ctx.res.statusCode = prevStatusCode; + return done(); + }); + + function done() { + // Express mutates the req object to make this property non-writable. + // We need to make it writable because other plugins (like koa-helmet) will set it // $FlowFixMe - ctx.res.end = oldEnd; + Object.defineProperty(req, 'secure', { + // $FlowFixMe + value: req.secure, + writable: true, + }); + res.removeListener('end', listener); + res.removeListener('finish', listener); return next() .then(resolve) .catch(reject); - }); + } }); }; },