Skip to content

Commit 502199e

Browse files
KrinkleKrinkle
committed
server: Avoid duplicate "testfile_read" log message, minor refactor
* Avoid duplicate "testfile_read" log message when reloading in debug mode. On reload, the `this.testFilePromise` will have settled already and so nothing really "finishes" that time. Move this to fetchTestFile() instead, which runs only one. This has the added benefit of reliably logging it when it truly finishes, instead of when we look for it (possibly well after it originally finished). * Move isURL to util. * Re-order class methods a bit to be more natural imho.
1 parent 6819f50 commit 502199e

File tree

2 files changed

+173
-173
lines changed

2 files changed

+173
-173
lines changed

‎src/server.js‎

Lines changed: 169 additions & 173 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class ControlServer {
3434
let root = options.cwd;
3535
let testFileAbsolute;
3636
let testFileQueryString = '';
37-
if (this.isURL(testFile)) {
37+
if (util.isURL(testFile)) {
3838
testFileAbsolute = testFile;
3939
} else {
4040
// For `qtap ../foobar/test/index.html`, default root to ../foobar.
@@ -153,25 +153,181 @@ class ControlServer {
153153

154154
/** @return {Promise<Object>} Headers and HTML document */
155155
async fetchTestFile (file) {
156+
let headers, body;
157+
156158
// fetch() does not support file URLs (as of writing, in Node.js 22).
157-
if (this.isURL(file)) {
158-
this.logger.debug('testfile_fetch', `Requesting a copy of ${file}`);
159+
if (util.isURL(file)) {
160+
this.logger.debug('testfile_fetch', `Requesting ${file}`);
159161
const resp = await fetch(file);
160162
if (!resp.ok) {
161163
throw new Error('Remote URL responded with HTTP ' + resp.status);
162164
}
163-
return {
164-
// TODO: Write a test to confirm that we preserve response headers
165-
headers: resp.headers,
166-
body: await resp.text()
167-
};
165+
// TODO: Write a test to confirm that we preserve response headers
166+
headers = resp.headers;
167+
body = await resp.text();
168168
} else {
169-
this.logger.debug('testfile_read', `Reading file contents from ${file}`);
170-
return {
171-
headers: new Headers(),
172-
body: (await fsPromises.readFile(file)).toString()
173-
};
169+
this.logger.debug('testfile_read', `Reading file ${file}`);
170+
headers = new Headers();
171+
body = (await fsPromises.readFile(file)).toString();
172+
}
173+
174+
this.logger.debug('testfile_ready', `Finished fetching ${file}`);
175+
return { headers, body };
176+
}
177+
178+
async getTestFile (clientId) {
179+
const proxyBase = await this.getProxyBase();
180+
const qtapTapUrl = proxyBase + '/.qtap/tap/?qtap_clientId=' + clientId;
181+
182+
let headInjectHtml = `<script>(${util.fnToStr(qtapClientHead, qtapTapUrl)})();</script>`;
183+
184+
// Add <base> tag so that URL-based files can fetch their resources directly from the
185+
// original server. Prepend as early as possible. If the file has its own <base>, theirs
186+
// will be after ours and correctly "win" by applying last.
187+
if (util.isURL(this.testFile)) {
188+
headInjectHtml = `<base href="${util.escapeHTML(this.testFile)}"/>` + headInjectHtml;
189+
}
190+
191+
let resp;
192+
let html;
193+
try {
194+
resp = await this.testFilePromise;
195+
html = resp.body;
196+
} catch (e) {
197+
// @ts-ignore - TypeScript @types/node lacks `Error(,options)`
198+
throw new Error('Could not open ' + this.testFile, { cause: e });
199+
}
200+
201+
// Head injection
202+
// * Use a callback, to avoid corruption if "$1" appears in the user input.
203+
// * The headInjectHtml string must be one line without any line breaks,
204+
// so that line numbers in stack traces presented in QTap output remain
205+
// transparent and correct.
206+
// * Ignore <heading> and <head-thing>.
207+
// * Support <head x=y...>, including with tabs or newlines before ">".
208+
html = util.replaceOnce(html,
209+
[
210+
/<head(?:\s[^>]*)?>/i,
211+
/<html(?:\s[^>]*)?>/i,
212+
/<!doctype[^>]*>/i,
213+
/^/
214+
],
215+
(m) => m + headInjectHtml
216+
);
217+
218+
html = util.replaceOnce(html,
219+
[
220+
/<\/body>(?![\s\S]*<\/body>)/i,
221+
/<\/html>(?![\s\S]*<\/html>)/i,
222+
/$/
223+
],
224+
(m) => '<script>(' + util.fnToStr(qtapClientBody, qtapTapUrl) + ')();</script>' + m
225+
);
226+
227+
return {
228+
headers: resp.headers,
229+
body: html
230+
};
231+
}
232+
233+
async handleRequest (req, url, resp) {
234+
const filePath = path.join(this.root, url.pathname);
235+
const ext = path.extname(url.pathname).slice(1);
236+
if (!filePath.startsWith(this.root)) {
237+
// Disallow outside directory traversal
238+
this.logger.debug('respond_static_deny', url.pathname);
239+
return this.serveError(resp, 403, 'HTTP 403: QTap respond_static_deny');
240+
}
241+
242+
const clientId = url.searchParams.get('qtap_clientId');
243+
if (clientId !== null) {
244+
// Serve the testfile from any URL path, as chosen by launchBrowser()
245+
const browser = this.browsers.get(clientId);
246+
if (browser) {
247+
browser.logger.debug('browser_connected', `${browser.getDisplayName()} connected! Serving test file.`);
248+
this.eventbus.emit('online', { clientId });
249+
} else if (this.debugMode) {
250+
// Allow users to reload the page when in --debug mode.
251+
// Note that do not handle more TAP results after a given test run has finished.
252+
this.logger.debug('browser_reload_debug', clientId);
253+
} else {
254+
this.logger.debug('browser_unknown_clientId', clientId);
255+
return this.serveError(resp, 403, 'HTTP 403: QTap browser_unknown_clientId.\n\nThis clientId was likely already served and cannot be repeated. Run qtap with --debug to bypass this restriction.');
256+
}
257+
258+
const testFileResp = await this.getTestFile(clientId);
259+
for (const [name, value] of testFileResp.headers) {
260+
resp.setHeader(name, value);
261+
}
262+
if (!testFileResp.headers.get('Content-Type')) {
263+
resp.setHeader('Content-Type', util.MIME_TYPES.html);
264+
}
265+
resp.writeHead(200);
266+
resp.write(testFileResp.body);
267+
resp.end();
268+
269+
// Count proxying the test file toward connectTimeout, not idleTimeout.
270+
if (browser) {
271+
browser.clientIdleActive = performance.now();
272+
}
273+
return;
274+
}
275+
276+
if (!fs.existsSync(filePath)) {
277+
this.logger.debug('respond_static_notfound', filePath);
278+
return this.serveError(resp, 404, 'HTTP 404: QTap respond_static_notfound');
279+
}
280+
281+
this.logger.debug('respond_static_pipe', filePath);
282+
resp.writeHead(200, { 'Content-Type': util.MIME_TYPES[ext] || util.MIME_TYPES.bin });
283+
fs.createReadStream(filePath)
284+
.on('error', (err) => {
285+
this.logger.warning('respond_static_pipe_error', err);
286+
resp.end();
287+
})
288+
.pipe(resp);
289+
}
290+
291+
handleTap (req, url, resp) {
292+
let body = '';
293+
req.on('data', (data) => {
294+
body += data;
295+
});
296+
req.on('end', () => {
297+
// Support QUnit 2.16 - 2.23: Strip escape sequences for tap-parser compatibility.
298+
// Fixed in QUnit 2.23.1 with https://github.com/qunitjs/qunit/pull/1801.
299+
body = util.stripAsciEscapes(body);
300+
const bodyExcerpt = body.slice(0, 30) + '…';
301+
const clientId = url.searchParams.get('qtap_clientId');
302+
const browser = this.browsers.get(clientId);
303+
if (browser) {
304+
const now = performance.now();
305+
browser.logger.debug('browser_tap_received',
306+
`+${util.humanSeconds(now - browser.clientIdleActive)}s`,
307+
bodyExcerpt
308+
);
309+
browser.tapParser.write(body);
310+
browser.clientIdleActive = now;
311+
} else {
312+
this.logger.debug('browser_tap_unhandled', clientId, bodyExcerpt);
313+
}
314+
});
315+
resp.writeHead(204);
316+
resp.end();
317+
}
318+
319+
/**
320+
* @param {http.ServerResponse} resp
321+
* @param {number} statusCode
322+
* @param {string|Error} e
323+
*/
324+
serveError (resp, statusCode, e) {
325+
if (!resp.headersSent) {
326+
resp.writeHead(statusCode, { 'Content-Type': util.MIME_TYPES.txt });
327+
// @ts-ignore - TypeScript @types/node lacks Error.stack
328+
resp.write((e.stack || String(e)) + '\n');
174329
}
330+
resp.end();
175331
}
176332

177333
async launchBrowser (browserFn, browserName, globalSignal) {
@@ -401,169 +557,9 @@ class ControlServer {
401557
return result;
402558
}
403559

404-
async getTestFile (clientId) {
405-
const proxyBase = await this.getProxyBase();
406-
const qtapTapUrl = proxyBase + '/.qtap/tap/?qtap_clientId=' + clientId;
407-
408-
let headInjectHtml = `<script>(${util.fnToStr(qtapClientHead, qtapTapUrl)})();</script>`;
409-
410-
// Add <base> tag so that URL-based files can fetch their resources directly from the
411-
// original server. Prepend as early as possible. If the file has its own <base>, theirs
412-
// will be after ours and correctly "win" by applying last.
413-
if (this.isURL(this.testFile)) {
414-
headInjectHtml = `<base href="${util.escapeHTML(this.testFile)}"/>` + headInjectHtml;
415-
}
416-
417-
let resp;
418-
let html;
419-
try {
420-
resp = await this.testFilePromise;
421-
html = resp.body;
422-
} catch (e) {
423-
// @ts-ignore - TypeScript @types/node lacks `Error(,options)`
424-
throw new Error('Could not open ' + this.testFile, { cause: e });
425-
}
426-
this.logger.debug('testfile_ready', `Finished fetching ${this.testFile}`);
427-
428-
// Head injection
429-
// * Use a callback, to avoid corruption if "$1" appears in the user input.
430-
// * The headInjectHtml string must be one line without any line breaks,
431-
// so that line numbers in stack traces presented in QTap output remain
432-
// transparent and correct.
433-
// * Ignore <heading> and <head-thing>.
434-
// * Support <head x=y...>, including with tabs or newlines before ">".
435-
html = util.replaceOnce(html,
436-
[
437-
/<head(?:\s[^>]*)?>/i,
438-
/<html(?:\s[^>]*)?>/i,
439-
/<!doctype[^>]*>/i,
440-
/^/
441-
],
442-
(m) => m + headInjectHtml
443-
);
444-
445-
html = util.replaceOnce(html,
446-
[
447-
/<\/body>(?![\s\S]*<\/body>)/i,
448-
/<\/html>(?![\s\S]*<\/html>)/i,
449-
/$/
450-
],
451-
(m) => '<script>(' + util.fnToStr(qtapClientBody, qtapTapUrl) + ')();</script>' + m
452-
);
453-
454-
return {
455-
headers: resp.headers,
456-
body: html
457-
};
458-
}
459-
460-
async handleRequest (req, url, resp) {
461-
const filePath = path.join(this.root, url.pathname);
462-
const ext = path.extname(url.pathname).slice(1);
463-
if (!filePath.startsWith(this.root)) {
464-
// Disallow outside directory traversal
465-
this.logger.debug('respond_static_deny', url.pathname);
466-
return this.serveError(resp, 403, 'HTTP 403: QTap respond_static_deny');
467-
}
468-
469-
const clientId = url.searchParams.get('qtap_clientId');
470-
if (clientId !== null) {
471-
// Serve the testfile from any URL path, as chosen by launchBrowser()
472-
const browser = this.browsers.get(clientId);
473-
if (browser) {
474-
browser.logger.debug('browser_connected', `${browser.getDisplayName()} connected! Serving test file.`);
475-
this.eventbus.emit('online', { clientId });
476-
} else if (this.debugMode) {
477-
// Allow users to reload the page when in --debug mode.
478-
// Note that do not handle more TAP results after a given test run has finished.
479-
this.logger.debug('browser_reload_debug', clientId);
480-
} else {
481-
this.logger.debug('browser_unknown_clientId', clientId);
482-
return this.serveError(resp, 403, 'HTTP 403: QTap browser_unknown_clientId.\n\nThis clientId was likely already served and cannot be repeated. Run qtap with --debug to bypass this restriction.');
483-
}
484-
485-
const testFileResp = await this.getTestFile(clientId);
486-
for (const [name, value] of testFileResp.headers) {
487-
resp.setHeader(name, value);
488-
}
489-
if (!testFileResp.headers.get('Content-Type')) {
490-
resp.setHeader('Content-Type', util.MIME_TYPES.html);
491-
}
492-
resp.writeHead(200);
493-
resp.write(testFileResp.body);
494-
resp.end();
495-
496-
// Count proxying the test file toward connectTimeout, not idleTimeout.
497-
if (browser) {
498-
browser.clientIdleActive = performance.now();
499-
}
500-
return;
501-
}
502-
503-
if (!fs.existsSync(filePath)) {
504-
this.logger.debug('respond_static_notfound', filePath);
505-
return this.serveError(resp, 404, 'HTTP 404: QTap respond_static_notfound');
506-
}
507-
508-
this.logger.debug('respond_static_pipe', filePath);
509-
resp.writeHead(200, { 'Content-Type': util.MIME_TYPES[ext] || util.MIME_TYPES.bin });
510-
fs.createReadStream(filePath)
511-
.on('error', (err) => {
512-
this.logger.warning('respond_static_pipe_error', err);
513-
resp.end();
514-
})
515-
.pipe(resp);
516-
}
517-
518-
handleTap (req, url, resp) {
519-
let body = '';
520-
req.on('data', (data) => {
521-
body += data;
522-
});
523-
req.on('end', () => {
524-
// Support QUnit 2.16 - 2.23: Strip escape sequences for tap-parser compatibility.
525-
// Fixed in QUnit 2.23.1 with https://github.com/qunitjs/qunit/pull/1801.
526-
body = util.stripAsciEscapes(body);
527-
const bodyExcerpt = body.slice(0, 30) + '…';
528-
const clientId = url.searchParams.get('qtap_clientId');
529-
const browser = this.browsers.get(clientId);
530-
if (browser) {
531-
const now = performance.now();
532-
browser.logger.debug('browser_tap_received',
533-
`+${util.humanSeconds(now - browser.clientIdleActive)}s`,
534-
bodyExcerpt
535-
);
536-
browser.tapParser.write(body);
537-
browser.clientIdleActive = now;
538-
} else {
539-
this.logger.debug('browser_tap_unhandled', clientId, bodyExcerpt);
540-
}
541-
});
542-
resp.writeHead(204);
543-
resp.end();
544-
}
545-
546-
/**
547-
* @param {http.ServerResponse} resp
548-
* @param {number} statusCode
549-
* @param {string|Error} e
550-
*/
551-
serveError (resp, statusCode, e) {
552-
if (!resp.headersSent) {
553-
resp.writeHead(statusCode, { 'Content-Type': util.MIME_TYPES.txt });
554-
// @ts-ignore - TypeScript @types/node lacks Error.stack
555-
resp.write((e.stack || String(e)) + '\n');
556-
}
557-
resp.end();
558-
}
559-
560560
async getProxyBase () {
561561
return this.proxyBase || await this.proxyBasePromise;
562562
}
563-
564-
isURL (file) {
565-
return file.startsWith('http:') || file.startsWith('https:');
566-
}
567563
}
568564

569565
export { ControlServer };

‎src/util.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ export function replaceOnce (input, patterns, replacement) {
8787
return input;
8888
}
8989

90+
export function isURL (file) {
91+
return file.startsWith('http:') || file.startsWith('https:');
92+
}
93+
9094
export class CommandNotFoundError extends Error {}
9195

9296
export class BrowserStopSignal extends Error {}

0 commit comments

Comments
 (0)