From 4ec92aec503fc3e3ddfab4096208f50825808be4 Mon Sep 17 00:00:00 2001 From: CountBleck Date: Tue, 30 Aug 2022 10:35:54 -0700 Subject: [PATCH 1/3] Remove redundant try block in Cyclone's fetch The error handling in the catch block led to a bug where the request's body was piped to Cyclone's response. However, since the mock request in the catch block doesn't have a body property, let alone a pipe method, that line throws an error. --- static/customBare.mjs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/static/customBare.mjs b/static/customBare.mjs index 038dc38..91afad7 100644 --- a/static/customBare.mjs +++ b/static/customBare.mjs @@ -51,15 +51,7 @@ async function fetchBare(url, res, req) { }, } - try { - var request = await fetch(url.href, options); - } catch (e) { - var request = { - text() { - return 'Error: ' + e; - }, - } - } + var request = await fetch(url.href, options); try { var contentType = request.headers.get('content-type') || 'application/javascript' From 4ff19c9fa5863cc8f8da6f9a0cd6a6625af417b0 Mon Sep 17 00:00:00 2001 From: CountBleck Date: Tue, 30 Aug 2022 15:24:48 -0700 Subject: [PATCH 2/3] Improve error handling in some cases In Cyclone's fetchBare(), the content type wasn't correct, and the status line should be "Internal Server Error", not just "Error". In the main request handler, I added similar error handling, just in case. --- app.js | 19 +++++++++++++------ static/customBare.mjs | 4 ++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/app.js b/app.js index e73bc69..499ef6e 100644 --- a/app.js +++ b/app.js @@ -19,12 +19,19 @@ const serve = new nodeStatic.Server(join( const server = http.createServer(); server.on('request', (request, response) => { - if (custombare.route(request, response)) return true; - - if (bareServer.shouldRoute(request)) { - bareServer.routeRequest(request, response); - } else { - serve.serve(request, response); + try { + if (custombare.route(request, response)) return true; + + if (bareServer.shouldRoute(request)) { + bareServer.routeRequest(request, response); + } else { + serve.serve(request, response); + } + } catch (e) { + response.writeHead(500, "Internal Server Error", { + "Content-Type": "text/plain" + }) + response.end(e.stack) } }); server.on('upgrade', (req, socket, head) => { diff --git a/static/customBare.mjs b/static/customBare.mjs index 91afad7..bc14db5 100644 --- a/static/customBare.mjs +++ b/static/customBare.mjs @@ -82,8 +82,8 @@ async function fetchBare(url, res, req) { request.body.pipe(res) } } catch (e) { - res.writeHead(500, 'Error', { - 'content-type': 'application/javascript' + res.writeHead(500, 'Internal Server Error', { + 'Content-Type': 'text/plain' }) res.end(e.stack); } From a9e3b27cb52a83e42edb08a95ae2d7a8d5bfd4f1 Mon Sep 17 00:00:00 2001 From: CountBleck Date: Tue, 30 Aug 2022 15:26:46 -0700 Subject: [PATCH 3/3] Add UNSAFE last-resort measure to prevent crashes This is really unsafe, according to Node.js's docs (see link below). Therefore, this handler is only added if the UNSAFE_CONTINUE environment variable has a truthy value. https://nodejs.org/api/process.html#warning-using-uncaughtexception-correctly --- app.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app.js b/app.js index 499ef6e..76fb2a9 100644 --- a/app.js +++ b/app.js @@ -44,4 +44,12 @@ server.on('upgrade', (req, socket, head) => { server.listen(PORT); +if (process.env.UNSAFE_CONTINUE) + process.on("uncaughtException", (err, origin) => { + console.error(`Critical error (${origin}):`) + console.error(err) + console.error("UNSAFELY CONTINUING EXECUTION") + console.error() + }) + console.log(`Server running at http://localhost:${PORT}/.`);