-
Notifications
You must be signed in to change notification settings - Fork 3
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
add minimal windows support #3
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
node_modules | ||
package-lock.json | ||
|
||
.vscode | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,7 @@ class Server extends EventEmitter { | |
this._process = undefined; | ||
this._exitHandlers = []; | ||
this._exit = undefined; | ||
this._useFdPass = process.env.MOJO_SERVER_STARTER_AVOID_FDPASS ? false : process.platform !== 'win32'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No Mojo specific code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And let the users decide if they want to use a file descriptor or not. Hidden magic only makes it harder to write portable code. |
||
} | ||
|
||
/** | ||
|
@@ -64,15 +65,18 @@ class Server extends EventEmitter { | |
* @param {object} [options] - Optional settings | ||
* @param {boolean} [options.stdout=false] - Forward server output from STDOUT to STDOUT | ||
* @param {boolean} [options.stderr=true] - Forward server output from STDERR to STDERR | ||
* @param {number} [options.connectTimeout=30000] - Max time to wait for server ready, in mS | ||
* @param {number} [options.retryTime=60] - Time to retry for server ready, in mS | ||
* @returns {Promise} | ||
*/ | ||
launch (cmd, args, options = {}) { | ||
if (typeof this._process !== 'undefined') throw new Error('Server already launched'); | ||
const stdout = typeof options.stdout !== 'undefined' ? options.stdout : false; | ||
const stderr = typeof options.stderr !== 'undefined' ? options.stderr : true; | ||
|
||
const proc = (this._process = spawn(cmd, args, { stdio: ['pipe', 'pipe', 'pipe', this._fd] })); | ||
|
||
const connectTimeout = typeof options.connectTimeout !== 'undefined' ? options.connectTimeout : 30000; | ||
const retryTime = typeof options.retryTime !== 'undefined' ? options.retryTime : 60; | ||
const spawnOptions = this._useFdPass ? { stdio: ['pipe', 'pipe', 'pipe', this._fd] } : undefined; | ||
const proc = (this._process = spawn(cmd, args, spawnOptions)); | ||
proc.on('error', e => this.emit('error', e)); | ||
if (stdout) proc.stdout.pipe(process.stdout); | ||
if (stderr) proc.stderr.pipe(process.stderr); | ||
|
@@ -86,7 +90,23 @@ class Server extends EventEmitter { | |
this.emit('exit', code, signal); | ||
}); | ||
|
||
return new Promise(resolve => this._srv.close(resolve)); | ||
return new Promise(resolve => this._srv.close( | ||
() => { | ||
if (this._useFdPass) resolve(); | ||
else { | ||
const now = new Date(); | ||
const timeToStop = new Date(now.getTime() + connectTimeout); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even my feedback on the unused |
||
const port = this.port; | ||
(function loop () { | ||
const connection = net.connect(port, resolve); | ||
connection.on('error', err => { | ||
if (err.code === 'ECONNREFUSED' && new Date() < timeToStop) setTimeout(loop, retryTime); | ||
else resolve(); // this is intented: don't reject, just stop delaying | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One of my requirements was that it rejects on errors. Are you sure you don't just want to make your own module? |
||
}); | ||
})(); | ||
} | ||
} | ||
)); | ||
} | ||
|
||
/** | ||
|
@@ -130,6 +150,15 @@ class Server extends EventEmitter { | |
const port = this.port; | ||
return `http://${address}:${port}`; | ||
} | ||
|
||
/** | ||
* Listen Address to configure service to be launched | ||
* @returns {string} | ||
*/ | ||
listenAddress () { | ||
if (this._useFdPass) return 'http://*?fd=3'; | ||
return this.url(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're still missing the point. This module is not meant to be Mojolicious specific at all. This could be a generic feature that any other web framework could use just the same. |
||
} | ||
|
||
exports = module.exports = new ServerStarter(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,8 +35,5 @@ | |
}, | ||
"engines": { | ||
"node": ">= 10" | ||
}, | ||
"os": [ | ||
"!win32" | ||
] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
'use strict'; | ||
|
||
// Usage: node server-starter.js <listen address> <listen delay (mS)> | ||
// <listen address> is mandatory | ||
|
||
const http = require('http'); | ||
const delay = process.argv[3] ? process.argv[3] : 0; | ||
let listen = {fd: 3}; | ||
let parts = process.argv[2].match(/http:\/\/([^\/]+):(\d+)/); | ||
if (parts) listen = {port: parts[2], address: parts[1]}; | ||
const server = http.createServer((req, res) => { | ||
res.writeHead(200, { 'Content-Type': 'text/plain' }); | ||
res.end('Hello World!'); | ||
}); | ||
|
||
// delayed start listening | ||
(() => new Promise(resolve => { | ||
setTimeout( | ||
() => resolve(server.listen(listen)), | ||
delay) | ||
}))(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had renamed the test server to |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to the rest of the PR.