From 7f90203a66d29c2dd29deb5522bb91c73f048d43 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Tue, 30 Jul 2019 17:06:27 -0700 Subject: [PATCH] Better async error handling --- modules/actions/serveBrowsePage.js | 7 +++-- modules/actions/serveDirectoryBrowser.js | 21 ++++++++----- modules/actions/serveDirectoryMetadata.js | 23 +++++++++----- modules/actions/serveFileBrowser.js | 16 +++++++--- modules/actions/serveFileMetadata.js | 23 +++++++++----- modules/actions/serveModule.js | 2 +- modules/middleware/findEntry.js | 37 ++++++++++++++--------- modules/middleware/validateVersion.js | 5 ++- modules/utils/asyncHandler.js | 14 +++++++++ 9 files changed, 102 insertions(+), 46 deletions(-) create mode 100644 modules/utils/asyncHandler.js diff --git a/modules/actions/serveBrowsePage.js b/modules/actions/serveBrowsePage.js index de720c0..9473c72 100644 --- a/modules/actions/serveBrowsePage.js +++ b/modules/actions/serveBrowsePage.js @@ -2,9 +2,10 @@ import { renderToString, renderToStaticMarkup } from 'react-dom/server'; import BrowseApp from '../client/browse/App.js'; import MainTemplate from '../templates/MainTemplate.js'; -import { getAvailableVersions } from '../utils/npm.js'; +import asyncHandler from '../utils/asyncHandler.js'; import getScripts from '../utils/getScripts.js'; import { createElement, createHTML } from '../utils/markup.js'; +import { getAvailableVersions } from '../utils/npm.js'; const doctype = ''; const globalURLs = @@ -20,7 +21,7 @@ const globalURLs = 'react-dom': '/react-dom@16.8.6/umd/react-dom.development.js' }; -export default async function serveBrowsePage(req, res) { +async function serveBrowsePage(req, res) { const availableVersions = await getAvailableVersions(req.packageName); const data = { packageName: req.packageName, @@ -51,3 +52,5 @@ export default async function serveBrowsePage(req, res) { }) .send(html); } + +export default asyncHandler(serveBrowsePage); diff --git a/modules/actions/serveDirectoryBrowser.js b/modules/actions/serveDirectoryBrowser.js index 557f41a..2a830f7 100644 --- a/modules/actions/serveDirectoryBrowser.js +++ b/modules/actions/serveDirectoryBrowser.js @@ -2,6 +2,7 @@ import path from 'path'; import gunzip from 'gunzip-maybe'; import tar from 'tar-stream'; +import asyncHandler from '../utils/asyncHandler.js'; import bufferStream from '../utils/bufferStream.js'; import getContentType from '../utils/getContentType.js'; import getIntegrity from '../utils/getIntegrity.js'; @@ -45,15 +46,19 @@ async function findMatchingEntries(stream, filename) { return; } - const content = await bufferStream(stream); + try { + const content = await bufferStream(stream); - entry.contentType = getContentType(entry.path); - entry.integrity = getIntegrity(content); - entry.size = content.length; + entry.contentType = getContentType(entry.path); + entry.integrity = getIntegrity(content); + entry.size = content.length; - entries[entry.path] = entry; + entries[entry.path] = entry; - next(); + next(); + } catch (error) { + next(error); + } }) .on('finish', () => { accept(entries); @@ -61,7 +66,7 @@ async function findMatchingEntries(stream, filename) { }); } -export default async function serveDirectoryBrowser(req, res) { +async function serveDirectoryBrowser(req, res) { const stream = await getPackage(req.packageName, req.packageVersion); const filename = req.filename.slice(0, -1) || '/'; @@ -79,3 +84,5 @@ export default async function serveDirectoryBrowser(req, res) { serveBrowsePage(req, res); } + +export default asyncHandler(serveDirectoryBrowser); diff --git a/modules/actions/serveDirectoryMetadata.js b/modules/actions/serveDirectoryMetadata.js index b1a589a..dbe9fc3 100644 --- a/modules/actions/serveDirectoryMetadata.js +++ b/modules/actions/serveDirectoryMetadata.js @@ -2,6 +2,7 @@ import path from 'path'; import gunzip from 'gunzip-maybe'; import tar from 'tar-stream'; +import asyncHandler from '../utils/asyncHandler.js'; import bufferStream from '../utils/bufferStream.js'; import getContentType from '../utils/getContentType.js'; import getIntegrity from '../utils/getIntegrity.js'; @@ -46,16 +47,20 @@ async function findMatchingEntries(stream, filename) { return; } - const content = await bufferStream(stream); + try { + const content = await bufferStream(stream); - entry.contentType = getContentType(entry.path); - entry.integrity = getIntegrity(content); - entry.lastModified = header.mtime.toUTCString(); - entry.size = content.length; + entry.contentType = getContentType(entry.path); + entry.integrity = getIntegrity(content); + entry.lastModified = header.mtime.toUTCString(); + entry.size = content.length; - entries[entry.path] = entry; + entries[entry.path] = entry; - next(); + next(); + } catch (error) { + next(error); + } }) .on('finish', () => { accept(entries); @@ -86,7 +91,7 @@ function getMetadata(entry, entries) { return metadata; } -export default async function serveDirectoryMetadata(req, res) { +async function serveDirectoryMetadata(req, res) { const stream = await getPackage(req.packageName, req.packageVersion); const filename = req.filename.slice(0, -1) || '/'; @@ -95,3 +100,5 @@ export default async function serveDirectoryMetadata(req, res) { res.send(metadata); } + +export default asyncHandler(serveDirectoryMetadata); diff --git a/modules/actions/serveFileBrowser.js b/modules/actions/serveFileBrowser.js index a63a033..ad021d1 100644 --- a/modules/actions/serveFileBrowser.js +++ b/modules/actions/serveFileBrowser.js @@ -1,6 +1,7 @@ import gunzip from 'gunzip-maybe'; import tar from 'tar-stream'; +import asyncHandler from '../utils/asyncHandler.js'; import bufferStream from '../utils/bufferStream.js'; import createDataURI from '../utils/createDataURI.js'; import getContentType from '../utils/getContentType.js'; @@ -37,10 +38,15 @@ async function findEntry(stream, filename) { return; } - entry.content = await bufferStream(stream); - foundEntry = entry; + try { + entry.content = await bufferStream(stream); - next(); + foundEntry = entry; + + next(); + } catch (error) { + next(error); + } }) .on('finish', () => { accept(foundEntry); @@ -48,7 +54,7 @@ async function findEntry(stream, filename) { }); } -export default async function serveFileBrowser(req, res) { +async function serveFileBrowser(req, res) { const stream = await getPackage(req.packageName, req.packageVersion); const entry = await findEntry(stream, req.filename); @@ -82,3 +88,5 @@ export default async function serveFileBrowser(req, res) { serveBrowsePage(req, res); } + +export default asyncHandler(serveFileBrowser); diff --git a/modules/actions/serveFileMetadata.js b/modules/actions/serveFileMetadata.js index f54c1bc..4cf1f4b 100644 --- a/modules/actions/serveFileMetadata.js +++ b/modules/actions/serveFileMetadata.js @@ -1,6 +1,7 @@ import gunzip from 'gunzip-maybe'; import tar from 'tar-stream'; +import asyncHandler from '../utils/asyncHandler.js'; import bufferStream from '../utils/bufferStream.js'; import getContentType from '../utils/getContentType.js'; import getIntegrity from '../utils/getIntegrity.js'; @@ -32,16 +33,20 @@ async function findEntry(stream, filename) { return; } - const content = await bufferStream(stream); + try { + const content = await bufferStream(stream); - entry.contentType = getContentType(entry.path); - entry.integrity = getIntegrity(content); - entry.lastModified = header.mtime.toUTCString(); - entry.size = content.length; + entry.contentType = getContentType(entry.path); + entry.integrity = getIntegrity(content); + entry.lastModified = header.mtime.toUTCString(); + entry.size = content.length; - foundEntry = entry; + foundEntry = entry; - next(); + next(); + } catch (error) { + next(error); + } }) .on('finish', () => { accept(foundEntry); @@ -49,7 +54,7 @@ async function findEntry(stream, filename) { }); } -export default async function serveFileMetadata(req, res) { +async function serveFileMetadata(req, res) { const stream = await getPackage(req.packageName, req.packageVersion); const entry = await findEntry(stream, req.filename); @@ -59,3 +64,5 @@ export default async function serveFileMetadata(req, res) { res.send(entry); } + +export default asyncHandler(serveFileMetadata); diff --git a/modules/actions/serveModule.js b/modules/actions/serveModule.js index 819a298..10c9039 100644 --- a/modules/actions/serveModule.js +++ b/modules/actions/serveModule.js @@ -1,7 +1,7 @@ import serveHTMLModule from './serveHTMLModule.js'; import serveJavaScriptModule from './serveJavaScriptModule.js'; -export default async function serveModule(req, res) { +export default function serveModule(req, res) { if (req.entry.contentType === 'application/javascript') { return serveJavaScriptModule(req, res); } diff --git a/modules/middleware/findEntry.js b/modules/middleware/findEntry.js index 930d287..498e5af 100644 --- a/modules/middleware/findEntry.js +++ b/modules/middleware/findEntry.js @@ -2,12 +2,13 @@ import path from 'path'; import gunzip from 'gunzip-maybe'; import tar from 'tar-stream'; +import asyncHandler from '../utils/asyncHandler.js'; +import bufferStream from '../utils/bufferStream.js'; import createPackageURL from '../utils/createPackageURL.js'; import createSearch from '../utils/createSearch.js'; -import { getPackage } from '../utils/npm.js'; -import getIntegrity from '../utils/getIntegrity.js'; import getContentType from '../utils/getContentType.js'; -import bufferStream from '../utils/bufferStream.js'; +import getIntegrity from '../utils/getIntegrity.js'; +import { getPackage } from '../utils/npm.js'; function fileRedirect(req, res, entry) { // Redirect to the file with the extension so it's @@ -123,20 +124,24 @@ function searchEntries(stream, filename) { } } - const content = await bufferStream(stream); + try { + const content = await bufferStream(stream); - entry.contentType = getContentType(entry.path); - entry.integrity = getIntegrity(content); - entry.lastModified = header.mtime.toUTCString(); - entry.size = content.length; + entry.contentType = getContentType(entry.path); + entry.integrity = getIntegrity(content); + entry.lastModified = header.mtime.toUTCString(); + entry.size = content.length; - // Set the content only for the foundEntry and - // discard the buffer for all others. - if (entry === foundEntry) { - entry.content = content; + // Set the content only for the foundEntry and + // discard the buffer for all others. + if (entry === foundEntry) { + entry.content = content; + } + + next(); + } catch (error) { + next(error); } - - next(); }) .on('finish', () => { accept({ @@ -153,7 +158,7 @@ function searchEntries(stream, filename) { * Fetch and search the archive to try and find the requested file. * Redirect to the "index" file if a directory was requested. */ -export default async function findEntry(req, res, next) { +async function findEntry(req, res, next) { const stream = await getPackage(req.packageName, req.packageVersion); const { foundEntry: entry, matchingEntries: entries } = await searchEntries( stream, @@ -201,3 +206,5 @@ export default async function findEntry(req, res, next) { next(); } + +export default asyncHandler(findEntry); diff --git a/modules/middleware/validateVersion.js b/modules/middleware/validateVersion.js index 6c78047..0a76322 100644 --- a/modules/middleware/validateVersion.js +++ b/modules/middleware/validateVersion.js @@ -1,3 +1,4 @@ +import asyncHandler from '../utils/asyncHandler.js'; import createPackageURL from '../utils/createPackageURL.js'; import { getPackageConfig, resolveVersion } from '../utils/npm.js'; @@ -18,7 +19,7 @@ function semverRedirect(req, res, newVersion) { * fetch the package config and add it to req.packageConfig. Redirect to * the resolved version number if necessary. */ -export default async function validateVersion(req, res, next) { +async function validateVersion(req, res, next) { const version = await resolveVersion(req.packageName, req.packageVersion); if (!version) { @@ -47,3 +48,5 @@ export default async function validateVersion(req, res, next) { next(); } + +export default asyncHandler(validateVersion); diff --git a/modules/utils/asyncHandler.js b/modules/utils/asyncHandler.js new file mode 100644 index 0000000..9ffc98c --- /dev/null +++ b/modules/utils/asyncHandler.js @@ -0,0 +1,14 @@ +/** + * Useful for wrapping `async` request handlers so they + * automatically propagate errors. + */ +export default function asyncHandler(handler) { + return (req, res, next) => { + Promise.resolve(handler(req, res, next)).catch(error => { + console.error(`Unexpected error in ${handler.name}!`); + console.error(error); + + next(error); + }); + }; +}