Fix temp file open error handling (#64)

- Fix error handling when temporary fail cannot be opened (e.g. because of access error). `catch` doesn't work since `open` always returns child process reference when `wait: false`. It should address #64
- Add `sme-result-` prefix to the temporary file.
- Add 2 app errors: `CannotCreateTempFile` and `CannotOpenTempFile` in order to separate creating and opening an temporary HTML file
- Update dependencies
- Prepare 2.0.0 version

package.json

@@ -1,6 +1,6 @@
 {
   "name": "source-map-explorer",
-  "version": "2.0.0-beta.0",
+  "version": "2.0.0",
   "description": "Analyze and debug space usage through source maps",
   "main": "dist/index.js",
   "bin": {
@@ -78,7 +78,7 @@
     "convert-source-map": "^1.6.0",
     "ejs": "^2.6.1",
     "escape-html": "^1.0.3",
-    "glob": "^7.1.3",
+    "glob": "^7.1.4",
     "lodash": "^4.17.11",
     "open": "^6.2.0",
     "source-map": "^0.7.3",
@@ -100,9 +100,8 @@
     "@types/convert-source-map": "^1.5.1",
     "@types/ejs": "^2.6.3",
     "@types/escape-html": "0.0.20",
-    "@types/fs-extra": "^5.0.5",
     "@types/glob": "^7.1.1",
-    "@types/lodash": "^4.14.123",
+    "@types/lodash": "^4.14.124",
     "@types/mocha": "^5.2.6",
     "@types/node": "^11.x",
     "@types/open": "^6.1.0",

src/api.ts

@@ -1,7 +1,7 @@
 import fs from 'fs';
 import path from 'path';
 import glob from 'glob';
-import { partition, flatMap } from 'lodash';
+import { partition, flatMap, isString } from 'lodash';
 
 import { generateHtml } from './html';
 import { exploreBundle, UNMAPPED_KEY } from './explore';
@@ -31,10 +31,7 @@ export async function explore(
   }
 
   // Separate bundles from file tokens
-  const [fileTokens, bundles] = partition(
-    bundlesAndFileTokens,
-    item => typeof item === 'string'
-  ) as [string[], Bundle[]];
+  const [fileTokens, bundles] = partition(bundlesAndFileTokens, isString);
 
   // Get bundles from file tokens
   bundles.push(...getBundles(fileTokens));
@@ -104,10 +101,10 @@ function getExploreResult(
   results: (ExploreBundleResult | ExploreErrorResult)[],
   options: ExploreOptions
 ): ExploreResult {
-  const [bundles, errors] = partition(results, result => 'files' in result) as [
-    ExploreBundleResult[],
-    ExploreErrorResult[]
-  ];
+  const [bundles, errors] = partition(
+    results,
+    (result): result is ExploreBundleResult => 'files' in result
+  );
 
   errors.push(...getPostExploreErrors(bundles));
 

src/app-error.ts

@@ -1,16 +1,17 @@
 import { formatPercent } from './helpers';
+import { ErrorCode } from './index';
 
 // If we need advanced error consider using https://github.com/joyent/node-verror
 export class AppError extends Error {
-  code?: string;
+  code?: ErrorCode;
   cause?: Error;
 
   constructor(errorContext: ErrorContext, error?: NodeJS.ErrnoException) {
     super();
 
     const message = getErrorMessage(errorContext);
 
-    this.message = error ? `${message} ${error.message}` : message;
+    this.message = error ? `${message}: ${error.message}` : message;
     this.code = errorContext.code;
 
     Error.captureStackTrace(this, AppError);
@@ -21,7 +22,7 @@ export const SOURCE_MAP_INFO_URL =
   'https://github.com/danvk/source-map-explorer/blob/master/README.md#generating-source-maps';
 
 interface CommonErrorContext {
-  code: 'NoBundles' | 'NoSourceMap' | 'CannotSaveFile' | 'Unknown';
+  code: 'NoBundles' | 'NoSourceMap' | 'CannotSaveFile' | 'CannotCreateTempFile' | 'Unknown';
 }
 
 interface OneSourceSourceMapErrorContext {
@@ -31,11 +32,21 @@ interface OneSourceSourceMapErrorContext {
 
 interface UnmappedBytesErrorContext {
   code: 'UnmappedBytes';
-  unmappedBytes: number;
   totalBytes: number;
+  unmappedBytes: number;
 }
 
-type ErrorContext = CommonErrorContext | OneSourceSourceMapErrorContext | UnmappedBytesErrorContext;
+interface CannotOpenTempFileErrorContext {
+  code: 'CannotOpenTempFile';
+  error: Buffer;
+  tempFile: string;
+}
+
+type ErrorContext =
+  | CommonErrorContext
+  | OneSourceSourceMapErrorContext
+  | UnmappedBytesErrorContext
+  | CannotOpenTempFileErrorContext;
 
 export function getErrorMessage(context: ErrorContext): string {
   switch (context.code) {
@@ -47,9 +58,7 @@ export function getErrorMessage(context: ErrorContext): string {
 See ${SOURCE_MAP_INFO_URL}`;
 
     case 'OneSourceSourceMap': {
-      const { filename } = context;
-
-      return `Your source map only contains one source (${filename}).
+      return `Your source map only contains one source (${context.filename}).
 This can happen if you use browserify+uglifyjs, for example, and don't set the --in-source-map flag to uglify.
 See ${SOURCE_MAP_INFO_URL}`;
     }
@@ -63,7 +72,18 @@ See ${SOURCE_MAP_INFO_URL}`;
     }
 
     case 'CannotSaveFile':
-      return 'Unable to save html to file';
+      return 'Unable to save HTML to file';
+
+    case 'CannotCreateTempFile':
+      return 'Unable to create a temporary HTML file';
+
+    case 'CannotOpenTempFile': {
+      const { error, tempFile } = context;
+
+      return `Unable to open web browser. ${error.toString().trim()}
+Either run with --html, --json, --tsv, --file, or view HTML for the visualization at:
+${tempFile}`;
+    }
 
     default:
       return 'Unknown error';

src/cli.ts

@@ -9,6 +9,7 @@ import { groupBy } from 'lodash';
 
 import { explore } from './api';
 import { ExploreOptions, ReplaceMap, FileSizeMap, ExploreResult } from './index';
+import { AppError, getErrorMessage } from './app-error';
 
 /** Parsed CLI arguments */
 interface Arguments {
@@ -204,17 +205,23 @@ function writeToHtml(html?: string): void {
     return;
   }
 
-  const tempName = temp.path({ suffix: '.html' });
-
-  fs.writeFileSync(tempName, html);
-
-  open(tempName, { wait: false }).catch(error => {
-    logError('Unable to open web browser.', error);
-    logError(
-      'Either run with --html, --json, --tsv, --file, or view HTML for the visualization at:'
-    );
-    logError(tempName);
-  });
+  try {
+    const info = temp.openSync({ prefix: 'sme-result-', suffix: '.html' });
+    fs.writeFileSync(info.fd, html);
+
+    open(info.path).then(child => {
+      if (child.stderr) {
+        // Catch error output from child process
+        child.stderr.once('data', (data: Buffer) => {
+          logError(
+            getErrorMessage({ code: 'CannotOpenTempFile', tempFile: info.path, error: data })
+          );
+        });
+      }
+    });
+  } catch (error) {
+    throw new AppError({ code: 'CannotCreateTempFile' }, error);
+  }
 }
 
 function outputErrors({ errors }: ExploreResult): void {

src/index.ts

@@ -20,7 +20,9 @@ export type ErrorCode =
   | 'NoSourceMap'
   | 'OneSourceSourceMap'
   | 'UnmappedBytes'
-  | 'CannotSaveFile';
+  | 'CannotSaveFile'
+  | 'CannotCreateTempFile'
+  | 'CannotOpenTempFile';
 
 export type File = string | Buffer;
 

tests/__snapshots__/app-error.test.ts.snap

@@ -0,0 +1,36 @@
+exports['app-error getErrorMessage should create message for \'CannotCreateTempFile\' 1'] = `
+Unable to create a temporary HTML file
+`
+
+exports['app-error getErrorMessage should create message for \'CannotOpenTempFile\' 1'] = `
+Unable to open web browser. The system cannot find the file ?C:\\foo.htm
+Either run with --html, --json, --tsv, --file, or view HTML for the visualization at:
+C:\\foo.htm
+`
+
+exports['app-error getErrorMessage should create message for \'CannotSaveFile\' 1'] = `
+Unable to save HTML to file
+`
+
+exports['app-error getErrorMessage should create message for \'NoBundles\' 1'] = `
+No file(s) provided
+`
+
+exports['app-error getErrorMessage should create message for \'NoSourceMap\' 1'] = `
+Unable to find a source map.
+See https://github.com/danvk/source-map-explorer/blob/master/README.md#generating-source-maps
+`
+
+exports['app-error getErrorMessage should create message for \'OneSourceSourceMap\' 1'] = `
+Your source map only contains one source (foo.min.js).
+This can happen if you use browserify+uglifyjs, for example, and don't set the --in-source-map flag to uglify.
+See https://github.com/danvk/source-map-explorer/blob/master/README.md#generating-source-maps
+`
+
+exports['app-error getErrorMessage should create message for \'UnknownCode\' 1'] = `
+Unknown error
+`
+
+exports['app-error getErrorMessage should create message for \'UnmappedBytes\' 1'] = `
+Unable to map 70/100 bytes (70.00%)
+`

tests/app-error.test.ts

@@ -0,0 +1,28 @@
+import snapshot from '@smpx/snap-shot-it';
+
+import { getErrorMessage } from '../src/app-error';
+
+describe('app-error', function() {
+  describe('getErrorMessage', function() {
+    const tests = [
+      { code: 'NoBundles' },
+      { code: 'NoSourceMap' },
+      { code: 'OneSourceSourceMap', filename: 'foo.min.js' },
+      { code: 'UnmappedBytes', totalBytes: 100, unmappedBytes: 70 },
+      { code: 'CannotSaveFile' },
+      { code: 'CannotCreateTempFile' },
+      {
+        code: 'CannotOpenTempFile',
+        error: Buffer.from('The system cannot find the file ?C:\\foo.htm'),
+        tempFile: 'C:\\foo.htm',
+      },
+      { code: 'UnknownCode' },
+    ];
+
+    tests.forEach(function(context) {
+      it(`should create message for '${context.code}'`, function() {
+        snapshot(getErrorMessage(context as any));
+      });
+    });
+  });
+});

tests/helpers.test.ts

@@ -2,7 +2,7 @@ import { expect } from 'chai';
 
 import { getCommonPathPrefix } from '../src/helpers';
 
-describe('helpers', () => {
+describe('helpers', function() {
   describe('getCommonPathPrefix', function() {
     const tests = [
       { paths: ['abc', 'abcd', 'ab'], expected: '' },