From 3f36abf71d6bc9218192e65155c91ce73f983f79 Mon Sep 17 00:00:00 2001 From: Joey Guerra Date: Sat, 3 Jan 2026 16:34:29 -0600 Subject: [PATCH 1/2] feat: Enable using something other than Express for a web server --- bin/Hubot.mjs | 18 +++++++++- src/Robot.mjs | 31 ++++++++++------- src/ports/ExpressHttpServerPort.mjs | 53 +++++++++++++++++++++++++++++ src/ports/HttpServerPort.mjs | 35 +++++++++++++++++++ test/Hubot_test.mjs | 2 +- test/Robot_test.mjs | 18 ++++++++++ test/contracts/HttpServer_test.mjs | 36 ++++++++++++++++++++ 7 files changed, 179 insertions(+), 14 deletions(-) create mode 100644 src/ports/ExpressHttpServerPort.mjs create mode 100644 src/ports/HttpServerPort.mjs create mode 100644 test/contracts/HttpServer_test.mjs diff --git a/bin/Hubot.mjs b/bin/Hubot.mjs index 529341131..00b662e9e 100755 --- a/bin/Hubot.mjs +++ b/bin/Hubot.mjs @@ -11,6 +11,7 @@ const switches = [ ['-f', '--file HUBOT_FILE', 'Path to adapter file, e.g. "./adapters/CustomAdapter.mjs"'], ['-c', '--create HUBOT_CREATE', 'Create a deployable hubot'], ['-d', '--disable-httpd HUBOT_HTTPD', 'Disable the HTTP server'], + ['-s', '--httpdserver HUBOT_HTTPD_SERVER', 'Specify a custom HTTP server module'], ['-h', '--help', 'Display the help information'], ['-l', '--alias HUBOT_ALIAS', "Enable replacing the robot's name with alias"], ['-n', '--name HUBOT_NAME', 'The name of the robot in chat'], @@ -28,7 +29,8 @@ const options = { scripts: process.env.HUBOT_SCRIPTS || [], name: process.env.HUBOT_NAME || 'Hubot', file: process.env.HUBOT_FILE, - configCheck: false + configCheck: false, + httpdServer: process.env.HUBOT_HTTPD_SERVER, } const Parser = new OptParse(switches) @@ -51,6 +53,10 @@ Parser.on('disable-httpd', opt => { options.enableHttpd = false }) +Parser.on('httpdserver', (opt, value) => { + options.httpdServer = value +}) + Parser.on('help', function (opt, value) { console.log(Parser.toString()) return process.exit(0) @@ -99,6 +105,16 @@ if (options.file) { options.adapter = options.file.split('/').pop().split('.')[0] } +if (options.enableHttpd === true && !options.httpdServer) { + options.httpdServer = 'src/ports/ExpressHttpServerPort.mjs' +} + +if (options.httpdServer) { + const httpdServerModulePath = pathResolve('.', options.httpdServer) + const httpdServerModule = await import(httpdServerModulePath) + options.enableHttpd = httpdServerModule.default +} + const robot = Hubot.loadBot(options.adapter, options.enableHttpd, options.name, options.alias) export default robot diff --git a/src/Robot.mjs b/src/Robot.mjs index 91147014f..da0bf23ff 100644 --- a/src/Robot.mjs +++ b/src/Robot.mjs @@ -11,6 +11,7 @@ import Response from './Response.mjs' import { Listener, TextListener } from './Listener.mjs' import Message from './Message.mjs' import Middleware from './Middleware.mjs' +import { HttpServerPort } from './ports/HttpServerPort.mjs' const File = fs.promises const HUBOT_DEFAULT_ADAPTERS = ['Campfire', 'Shell'] @@ -24,7 +25,7 @@ class Robot { // dispatch them to matching listeners. // // adapter - A String of the adapter name. - // httpd - A Boolean whether to enable the HTTP daemon. + // httpd - An async function that creates the HTTP daemon. Defaults to HttpServerPort, which doesn't do anything. // name - A String of the robot name, defaults to Hubot. // alias - A String of the alias of the robot name constructor (adapter, httpd, name, alias) { @@ -48,7 +49,17 @@ class Robot { this.adapterName = adapter ?? this.adapterName } - this.shouldEnableHttpd = httpd ?? true + this.httpDServerFactory = async (options) => new HttpServerPort(options) + if (httpd && typeof httpd === 'function') { + this.httpDServerFactory = httpd + } + + if (httpd === true) { + this.httpDServerFactory = Robot.setupExpress + } + + this.server = null + this.router = null this.datastore = null this.Response = Response this.commands = [] @@ -436,7 +447,7 @@ class Robot { // Setup the Express server's defaults. // // Returns Server. - async setupExpress () { + static async setupExpress (robot) { const user = process.env.EXPRESS_USER const pass = process.env.EXPRESS_PASSWORD const stat = process.env.EXPRESS_STATIC @@ -469,10 +480,10 @@ class Robot { } return new Promise((resolve, reject) => { try { - this.server = app.listen(port, address, () => { - this.router = app - this.emit('listening', this.server) - resolve(this.server) + robot.server = app.listen(port, address, () => { + robot.router = app + robot.emit('listening', robot.server) + resolve(robot.server) }) } catch (err) { reject(err) @@ -661,11 +672,7 @@ class Robot { // // Returns whatever the adapter returns. async run () { - if (this.shouldEnableHttpd) { - await this.setupExpress() - } else { - this.setupNullRouter() - } + await this.httpDServerFactory(this) await this.adapter.run() this.emit('running') } diff --git a/src/ports/ExpressHttpServerPort.mjs b/src/ports/ExpressHttpServerPort.mjs new file mode 100644 index 000000000..78933e107 --- /dev/null +++ b/src/ports/ExpressHttpServerPort.mjs @@ -0,0 +1,53 @@ +import { HttpServerPort } from './HttpServerPort.mjs' +import express from 'express' +import basicAuth from 'express-basic-auth' + +class ExpressHttpServerPort extends HttpServerPort { + constructor(robot) { + super(robot) + } + async start(port) { + const user = process.env.EXPRESS_USER + const pass = process.env.EXPRESS_PASSWORD + const stat = process.env.EXPRESS_STATIC + const address = process.env.EXPRESS_BIND_ADDRESS || process.env.BIND_ADDRESS || '0.0.0.0' + const limit = process.env.EXPRESS_LIMIT || '100kb' + const paramLimit = parseInt(process.env.EXPRESS_PARAMETER_LIMIT) || 1000 + const app = express() + + app.use((req, res, next) => { + res.setHeader('X-Powered-By', `hubot/${encodeURI(this.name)}`) + return next() + }) + + if (user && pass) { + const authUser = {} + authUser[user] = pass + app.use(basicAuth({ users: authUser })) + } + + app.use(express.json({ limit })) + app.use(express.urlencoded({ limit, parameterLimit: paramLimit, extended: true })) + + if (stat) { + app.use(express.static(stat)) + } + return new Promise((resolve, reject) => { + try { + this.robot.server = app.listen(port, address, () => { + this.robot.router = app + this.robot.emit('listening', this.robot.server) + resolve(this.robot.server) + }) + } catch (err) { + reject(err) + } + }) + } +} + +export default async (robot) => { + const server = new ExpressHttpServerPort(robot) + const port = process.env.EXPRESS_PORT || process.env.PORT || 8080 + return await server.start(port) +} diff --git a/src/ports/HttpServerPort.mjs b/src/ports/HttpServerPort.mjs new file mode 100644 index 000000000..396b706c7 --- /dev/null +++ b/src/ports/HttpServerPort.mjs @@ -0,0 +1,35 @@ +function randomPort() { + return Math.floor(Math.random() * (65535 - 1024)) + 1024 +} +class HttpServerPort { + constructor(robot) { + this.robot = robot + } + async stop() {} + async start(port) { + if (port === 0) { + port = randomPort() + } + this.port = port + this.robot.server = {} + this.robot.router = this + return this.robot.server + } + address() { + return { port: this.port } + } + use(handler) {} + get(path, handler) {} + post(path, handler) {} + put(path, handler) {} + patch(path, handler) {} + query(path, handler) {} + delete(path, handler) {} + trace(path, handler) {} + options(path, handler) {} + head(path, handler) {} +} + +export { + HttpServerPort +} \ No newline at end of file diff --git a/test/Hubot_test.mjs b/test/Hubot_test.mjs index 827cfbcfc..675122055 100644 --- a/test/Hubot_test.mjs +++ b/test/Hubot_test.mjs @@ -42,6 +42,7 @@ describe('Running bin/Hubot.mjs', () => { -f, --file HUBOT_FILE -c, --create HUBOT_CREATE -d, --disable-httpd HUBOT_HTTPD + -s, --httpdserver HUBOT_HTTPD_SERVER -h, --help -l, --alias HUBOT_ALIAS -n, --name HUBOT_NAME @@ -112,7 +113,6 @@ describe('Running hubot with args', () => { } finally { hubot.kill() } - console.log(actual) assert.ok(actual instanceof TypeError) assert.deepEqual(actual.message, 'fetch failed', 'this is an expected failure since the web service should not be running') done() diff --git a/test/Robot_test.mjs b/test/Robot_test.mjs index 1fb96d247..493239ff5 100644 --- a/test/Robot_test.mjs +++ b/test/Robot_test.mjs @@ -3,8 +3,11 @@ import { describe, it, beforeEach, afterEach } from 'node:test' import assert from 'node:assert/strict' import path from 'node:path' +import http from 'node:http' import { Robot, CatchAllMessage, EnterMessage, LeaveMessage, TextMessage, TopicMessage, User, Response } from '../index.mjs' import mockAdapter from './fixtures/MockAdapter.mjs' +import { HttpServerPort } from '../src/ports/HttpServerPort.mjs' +import { ExpressHttpServerPort } from '../src/ports/ExpressHttpServerPort.mjs' describe('Robot', () => { describe('#http', () => { @@ -1041,4 +1044,19 @@ describe('Robot', () => { delete process.env.PORT }) }) + + describe('Non-express HTTP server', async () => { + it('should work with a non-express http server', async () => { + + const robot = new Robot(mockAdapter, httpDServerFactory, 'TestHubot') + await robot.loadAdapter() + await robot.run() + const port = robot.server.address().port + const res = await fetch(`http://127.0.0.1:${port}/`) + assert.equal(res.status, 200) + assert.equal(await res.text(), 'Hello World\n') + robot.shutdown() + delete process.env.PORT + }) + }) }) diff --git a/test/contracts/HttpServer_test.mjs b/test/contracts/HttpServer_test.mjs new file mode 100644 index 000000000..cbcf2c421 --- /dev/null +++ b/test/contracts/HttpServer_test.mjs @@ -0,0 +1,36 @@ +import test from 'node:test' +import assert from 'node:assert/strict' +import { HttpServerPort } from '../../src/ports/HttpServerPort.mjs' + +test('HttpServerPort API', async t => { + await t.test('start and stop', async () => { + const server = new HttpServerPort({}) + await server.start(0) + assert.ok(server.address().port > 0, 'Server should be listening on a port') + await server.stop() + }) + + await t.test('use', async () => { + const server = new HttpServerPort({}) + server.use((req, res) => { + res.end('Hello, World!') + }) + await server.start(0) + await server.stop() + }) + + await t.test('http methods', async () => { + const server = new HttpServerPort({}) + await server.start(0) + assert.ok(server.get) + assert.ok(server.post) + assert.ok(server.put) + assert.ok(server.patch) + assert.ok(server.query) + assert.ok(server.delete) + assert.ok(server.trace) + assert.ok(server.options) + assert.ok(server.head) + await server.stop() + }) +}) \ No newline at end of file From 8c38749c4e7c7903b74ffc2d0681700b87840805 Mon Sep 17 00:00:00 2001 From: Joey Guerra Date: Sat, 3 Jan 2026 16:59:07 -0600 Subject: [PATCH 2/2] resolve co-pilot suggestions --- bin/Hubot.mjs | 2 +- src/ports/ExpressHttpServerPort.mjs | 23 +++++++++++++++++++---- src/ports/HttpServerPort.mjs | 6 ------ test/Robot_test.mjs | 26 ++++++++++++++++++++++++-- test/contracts/HttpServer_test.mjs | 2 +- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/bin/Hubot.mjs b/bin/Hubot.mjs index 00b662e9e..d91b31488 100755 --- a/bin/Hubot.mjs +++ b/bin/Hubot.mjs @@ -11,7 +11,7 @@ const switches = [ ['-f', '--file HUBOT_FILE', 'Path to adapter file, e.g. "./adapters/CustomAdapter.mjs"'], ['-c', '--create HUBOT_CREATE', 'Create a deployable hubot'], ['-d', '--disable-httpd HUBOT_HTTPD', 'Disable the HTTP server'], - ['-s', '--httpdserver HUBOT_HTTPD_SERVER', 'Specify a custom HTTP server module'], + ['-s', '--httpdserver HUBOT_HTTPD_SERVER', 'An async factory function that receives this robot (via options) and returns or creates an HttpServerPort instance. Defaults to a factory that instantiates HttpServerPort'], ['-h', '--help', 'Display the help information'], ['-l', '--alias HUBOT_ALIAS', "Enable replacing the robot's name with alias"], ['-n', '--name HUBOT_NAME', 'The name of the robot in chat'], diff --git a/src/ports/ExpressHttpServerPort.mjs b/src/ports/ExpressHttpServerPort.mjs index 78933e107..15282b758 100644 --- a/src/ports/ExpressHttpServerPort.mjs +++ b/src/ports/ExpressHttpServerPort.mjs @@ -3,8 +3,8 @@ import express from 'express' import basicAuth from 'express-basic-auth' class ExpressHttpServerPort extends HttpServerPort { - constructor(robot) { - super(robot) + get address() { + return this.robot.server.address() } async start(port) { const user = process.env.EXPRESS_USER @@ -16,7 +16,7 @@ class ExpressHttpServerPort extends HttpServerPort { const app = express() app.use((req, res, next) => { - res.setHeader('X-Powered-By', `hubot/${encodeURI(this.name)}`) + res.setHeader('X-Powered-By', `hubot/${encodeURI(this.robot.name)}`) return next() }) @@ -44,10 +44,25 @@ class ExpressHttpServerPort extends HttpServerPort { } }) } + + async stop() { + if (!this.robot || !this.robot.server) { + return + } + return new Promise((resolve, reject) => { + this.robot.server.close(err => { + if (err) { + return reject(err) + } + resolve() + }) + }) + } } export default async (robot) => { const server = new ExpressHttpServerPort(robot) const port = process.env.EXPRESS_PORT || process.env.PORT || 8080 - return await server.start(port) + await server.start(port) + return server } diff --git a/src/ports/HttpServerPort.mjs b/src/ports/HttpServerPort.mjs index 396b706c7..3ae757e7b 100644 --- a/src/ports/HttpServerPort.mjs +++ b/src/ports/HttpServerPort.mjs @@ -1,15 +1,9 @@ -function randomPort() { - return Math.floor(Math.random() * (65535 - 1024)) + 1024 -} class HttpServerPort { constructor(robot) { this.robot = robot } async stop() {} async start(port) { - if (port === 0) { - port = randomPort() - } this.port = port this.robot.server = {} this.robot.router = this diff --git a/test/Robot_test.mjs b/test/Robot_test.mjs index 493239ff5..c1b3c0b46 100644 --- a/test/Robot_test.mjs +++ b/test/Robot_test.mjs @@ -7,7 +7,6 @@ import http from 'node:http' import { Robot, CatchAllMessage, EnterMessage, LeaveMessage, TextMessage, TopicMessage, User, Response } from '../index.mjs' import mockAdapter from './fixtures/MockAdapter.mjs' import { HttpServerPort } from '../src/ports/HttpServerPort.mjs' -import { ExpressHttpServerPort } from '../src/ports/ExpressHttpServerPort.mjs' describe('Robot', () => { describe('#http', () => { @@ -1047,7 +1046,30 @@ describe('Robot', () => { describe('Non-express HTTP server', async () => { it('should work with a non-express http server', async () => { - + class HttpServer extends HttpServerPort { + async start(port) { + return new Promise((resolve, reject) => { + try { + this.robot.server = http.createServer((req, res) => { + res.writeHead(200, { 'Content-Type': 'text/plain' }) + res.end('Hello World\n') + }) + this.robot.router = this + this.robot.server = this.robot.server.listen(port, '127.0.0.1', () => { + this.robot.emit('listening', this.robot.server) + resolve(this.robot.server) + }) + } catch (err) { + reject(err) + } + }) + } + } + const httpDServerFactory = async (robot) => { + const server = new HttpServer(robot) + await server.start(process.env.PORT || 0) + return server + } const robot = new Robot(mockAdapter, httpDServerFactory, 'TestHubot') await robot.loadAdapter() await robot.run() diff --git a/test/contracts/HttpServer_test.mjs b/test/contracts/HttpServer_test.mjs index cbcf2c421..91d57018e 100644 --- a/test/contracts/HttpServer_test.mjs +++ b/test/contracts/HttpServer_test.mjs @@ -6,7 +6,7 @@ test('HttpServerPort API', async t => { await t.test('start and stop', async () => { const server = new HttpServerPort({}) await server.start(0) - assert.ok(server.address().port > 0, 'Server should be listening on a port') + assert.equal(server.address().port, 0) await server.stop() })