From ccea37256e990da26135684bf23881b4a7b2885d Mon Sep 17 00:00:00 2001
From: George Kalpakas <kalpakas.g@gmail.com>
Date: Wed, 3 Jan 2018 23:42:55 +0200
Subject: [PATCH] refactor(aio): use one argument for `DocViewer` error
 reporting (#21293)

Pass one argument to `logger.error()` to improve error reporting in
environments that do not handle more than one arguments well (e.g.
Googlebot's web rendering service).

Related to #21272.

PR Close #21293
---
 .../doc-viewer/doc-viewer.component.spec.ts   | 24 +++++++++++++++----
 .../layout/doc-viewer/doc-viewer.component.ts |  3 ++-
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/aio/src/app/layout/doc-viewer/doc-viewer.component.spec.ts b/aio/src/app/layout/doc-viewer/doc-viewer.component.spec.ts
index f0f2e7f3bc..688bcab8d2 100644
--- a/aio/src/app/layout/doc-viewer/doc-viewer.component.spec.ts
+++ b/aio/src/app/layout/doc-viewer/doc-viewer.component.spec.ts
@@ -536,7 +536,7 @@ describe('DocViewerComponent', () => {
         expect(swapViewsSpy).not.toHaveBeenCalled();
         expect(docViewer.nextViewContainer.innerHTML).toBe('');
         expect(logger.output.error).toEqual([
-          ['[DocViewer]: Error preparing document \'foo\'.', error],
+          [`[DocViewer] Error preparing document 'foo': ${error.stack}`],
         ]);
       });
 
@@ -555,7 +555,7 @@ describe('DocViewerComponent', () => {
         expect(swapViewsSpy).not.toHaveBeenCalled();
         expect(docViewer.nextViewContainer.innerHTML).toBe('');
         expect(logger.output.error).toEqual([
-          ['[DocViewer]: Error preparing document \'bar\'.', error],
+          [`[DocViewer] Error preparing document 'bar': ${error.stack}`],
         ]);
       });
 
@@ -574,7 +574,7 @@ describe('DocViewerComponent', () => {
         expect(swapViewsSpy).not.toHaveBeenCalled();
         expect(docViewer.nextViewContainer.innerHTML).toBe('');
         expect(logger.output.error).toEqual([
-          ['[DocViewer]: Error preparing document \'baz\'.', error],
+          [`[DocViewer] Error preparing document 'baz': ${error.stack}`],
         ]);
       });
 
@@ -593,7 +593,23 @@ describe('DocViewerComponent', () => {
         expect(swapViewsSpy).toHaveBeenCalledTimes(1);
         expect(docViewer.nextViewContainer.innerHTML).toBe('');
         expect(logger.output.error).toEqual([
-          ['[DocViewer]: Error preparing document \'qux\'.', error],
+          [`[DocViewer] Error preparing document 'qux': ${error.stack}`],
+        ]);
+      });
+
+      it('when something fails with non-Error', async () => {
+        const error = 'Typical string error';
+        swapViewsSpy.and.callFake(() => {
+          expect(docViewer.nextViewContainer.innerHTML).not.toBe('');
+          throw error;
+        });
+
+        await doRender('Some content', 'qux');
+
+        expect(swapViewsSpy).toHaveBeenCalledTimes(1);
+        expect(docViewer.nextViewContainer.innerHTML).toBe('');
+        expect(logger.output.error).toEqual([
+          [`[DocViewer] Error preparing document 'qux': ${error}`],
         ]);
       });
     });
diff --git a/aio/src/app/layout/doc-viewer/doc-viewer.component.ts b/aio/src/app/layout/doc-viewer/doc-viewer.component.ts
index cfb84e88ba..a798762c64 100644
--- a/aio/src/app/layout/doc-viewer/doc-viewer.component.ts
+++ b/aio/src/app/layout/doc-viewer/doc-viewer.component.ts
@@ -153,8 +153,9 @@ export class DocViewerComponent implements DoCheck, OnDestroy {
         .switchMap(() => this.swapViews(addTitleAndToc))
         .do(() => this.docRendered.emit())
         .catch(err => {
+          const errorMessage = (err instanceof Error) ? err.stack : err;
+          this.logger.error(`[DocViewer] Error preparing document '${doc.id}': ${errorMessage}`);
           this.nextViewContainer.innerHTML = '';
-          this.logger.error(`[DocViewer]: Error preparing document '${doc.id}'.`, err);
           return this.void$;
         });
   }