From 5a663b4ae33af55a148a9a749db11091fb1a9c20 Mon Sep 17 00:00:00 2001
From: Milly <milly.ca@gmail.com>
Date: Sun, 15 Mar 2020 01:53:21 +0900
Subject: [PATCH] fix(logging): do not use JSON.stringify to Error object

the result of `JSON.stringify(new Error('msg'))` is `{}`.
pass the `Error` object to logger as it is.
---
 src/plugins.ts           |  2 +-
 src/test/plugins.test.ts | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/plugins.ts b/src/plugins.ts
index 8436e9948..a92ff71b1 100644
--- a/src/plugins.ts
+++ b/src/plugins.ts
@@ -154,7 +154,7 @@ export class PluginLoader {
         this.logger.info(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`)
         const result = this.requirePlugin(resolvedPath, moduleName)
         if (result.error) {
-            this.logger.error(`Failed to load module: ${JSON.stringify(result.error)}`)
+            this.logger.error('Failed to load module:', result.error)
             return undefined
         }
         return result.module
diff --git a/src/test/plugins.test.ts b/src/test/plugins.test.ts
index 98bd522d8..42917ed67 100644
--- a/src/test/plugins.test.ts
+++ b/src/test/plugins.test.ts
@@ -1,6 +1,7 @@
 import * as path from 'path'
 import * as sinon from 'sinon'
 import * as ts from 'typescript'
+import { NoopLogger } from '../logging'
 import { InMemoryFileSystem } from '../memfs'
 import { PluginLoader, PluginModule, PluginModuleFactory } from '../plugins'
 import { PluginSettings } from '../request-type'
@@ -73,5 +74,32 @@ describe('plugins', () => {
             sinon.assert.calledOnce(applyProxy)
             sinon.assert.calledWithExactly(applyProxy, pluginFactoryFunc, sinon.match(pluginOption))
         })
+
+        it('should log error messages', async () => {
+            const memfs = new InMemoryFileSystem('/')
+            const peerPackagesPath = path.resolve(__filename, '../../../../')
+            const peerPackagesUri = path2uri(peerPackagesPath)
+            memfs.add(
+                peerPackagesUri + '/node_modules/some-plugin/package.json',
+                '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}'
+            )
+            memfs.add(peerPackagesUri + '/node_modules/some-plugin/plugin.js', '')
+            const pluginSettings: PluginSettings = {
+                globalPlugins: ['some-plugin'],
+                allowLocalPluginLoads: false,
+                pluginProbeLocations: [],
+            }
+            const internalError = new Error('invalid')
+            const fakeRequire = (_path: string) => { throw internalError }
+            const logger = new NoopLogger() as NoopLogger & { error: sinon.SinonStub }
+            sinon.stub(logger, 'error')
+            const loader = new PluginLoader('/', memfs, pluginSettings, logger, memfs, fakeRequire)
+            const compilerOptions: ts.CompilerOptions = {}
+            const applyProxy = sinon.spy()
+            loader.loadPlugins(compilerOptions, applyProxy)
+            sinon.assert.notCalled(applyProxy)
+            sinon.assert.calledWith(logger.error, sinon.match('Failed'), internalError)
+            sinon.assert.calledWith(logger.error, sinon.match('some-plugin'))
+        })
     })
 })