From dd3b63aa0be384b0207975430898f39eb6c4956d Mon Sep 17 00:00:00 2001
From: Zach Bloomquist <github@chary.us>
Date: Fri, 14 Feb 2020 16:29:51 -0500
Subject: [PATCH] Update project error UI (#6432)

* update default title to "an unexpected error occurred"

* show stack trace in error, if avail

* add "copy to clipboard"

* wrap browser errors

* add tests

* improve md formatting

* update desktopgui tests

* update
---
 .../cypress/integration/error_message_spec.js | 38 ++++++++++++++----
 .../cypress/integration/settings_spec.js      |  5 ++-
 packages/desktop-gui/src/lib/ipc.js           |  1 +
 .../desktop-gui/src/project/error-message.jsx | 39 ++++++++++++++++++-
 .../desktop-gui/src/project/project-model.js  |  3 ++
 packages/server/lib/gui/events.coffee         | 11 +++++-
 .../server/test/unit/gui/events_spec.coffee   | 22 +++++++++++
 7 files changed, 106 insertions(+), 13 deletions(-)

diff --git a/packages/desktop-gui/cypress/integration/error_message_spec.js b/packages/desktop-gui/cypress/integration/error_message_spec.js
index bcb03e418499..8e2135576691 100644
--- a/packages/desktop-gui/cypress/integration/error_message_spec.js
+++ b/packages/desktop-gui/cypress/integration/error_message_spec.js
@@ -128,17 +128,41 @@ describe('Error Message', function () {
     this.start()
 
     cy.get('.error').contains('ReferenceError: alsdkjf is not defined')
-    cy.get('details').should('not.have.attr', 'open')
-    cy.get('details').click().should('have.attr', 'open')
-    cy.get('summary').should('contain', 'ReferenceError')
+    cy.get('details.details-body').should('not.have.attr', 'open')
+    cy.get('details.details-body').click().should('have.attr', 'open')
+    cy.get('details.details-body > summary').should('contain', 'ReferenceError')
   })
 
   it('doesn\'t show error details if not provided', function () {
     cy.stub(this.ipc, 'onProjectError').yields(null, this.err)
     this.start()
 
+    cy.get('details.details-body > summary').should('not.exist')
+  })
+
+  it('shows error stack trace if provided', function () {
+    const err = new Error('foo')
+
+    err.stack = 'bar'
+
+    cy.stub(this.ipc, 'onProjectError').yields(null, err)
+    this.start()
+
+    cy.get('.error').contains('foo')
+    cy.get('details.stacktrace').should('not.have.attr', 'open')
+    cy.get('details.stacktrace').click().should('have.attr', 'open')
+    cy.get('details.stacktrace').should('contain', 'bar')
+  })
+
+  it('doesn\'t show error stack trace if not provided', function () {
+    const err = new Error()
+
+    delete err.stack
+    cy.stub(this.ipc, 'onProjectError').yields(null, err)
+    this.start()
+
     cy.get('.error')
-    cy.get('summary').should('not.exist')
+    cy.get('details.stacktrace > summary').should('not.be.visible')
   })
 
   it('shows abbreviated error details if only one line', function () {
@@ -155,7 +179,7 @@ describe('Error Message', function () {
     this.start()
 
     cy.get('.error').contains('ReferenceError: alsdkjf is not defined')
-    cy.get('summary').should('not.exist')
+    cy.get('details.details-body > summary').should('not.exist')
   })
 
   it('opens links outside of electron', function () {
@@ -197,7 +221,7 @@ describe('Error Message', function () {
     this.ipc.openProject.rejects(this.detailsErr)
     this.start()
 
-    cy.get('details').click()
+    cy.get('details.details-body').click()
     cy.get('nav').should('be.visible')
     cy.get('footer').should('be.visible')
   })
@@ -207,7 +231,7 @@ describe('Error Message', function () {
     this.ipc.openProject.rejects(this.detailsErr)
     this.start()
 
-    cy.get('details').click()
+    cy.get('details.details-body').click()
     cy.contains('Try Again').should('be.visible')
     cy.get('.full-alert pre').should('have.css', 'overflow', 'auto')
   })
diff --git a/packages/desktop-gui/cypress/integration/settings_spec.js b/packages/desktop-gui/cypress/integration/settings_spec.js
index 1b6fa3b1287b..cc550c815f7f 100644
--- a/packages/desktop-gui/cypress/integration/settings_spec.js
+++ b/packages/desktop-gui/cypress/integration/settings_spec.js
@@ -490,6 +490,7 @@ describe('Settings', () => {
   describe('errors', () => {
     beforeEach(function () {
       this.err = {
+        title: 'Foo Title',
         message: 'Port \'2020\' is already in use.',
         name: 'Error',
         port: 2020,
@@ -514,11 +515,11 @@ describe('Settings', () => {
     })
 
     it('displays errors', () => {
-      cy.contains('Can\'t start server')
+      cy.contains('Foo Title')
     })
 
     it('displays config after error is fixed', function () {
-      cy.contains('Can\'t start server').then(() => {
+      cy.contains('Foo Title').then(() => {
         this.ipc.openProject.onCall(1).resolves(this.config)
 
         this.ipc.onConfigChanged.yield()
diff --git a/packages/desktop-gui/src/lib/ipc.js b/packages/desktop-gui/src/lib/ipc.js
index 5eb2512449c4..7866fcfe4e12 100644
--- a/packages/desktop-gui/src/lib/ipc.js
+++ b/packages/desktop-gui/src/lib/ipc.js
@@ -64,5 +64,6 @@ register('updater:run', false)
 register('window:open')
 register('window:close')
 register('onboarding:closed')
+register('set:clipboard:text')
 
 export default ipc
diff --git a/packages/desktop-gui/src/project/error-message.jsx b/packages/desktop-gui/src/project/error-message.jsx
index 20664f59aa2e..ee974370a7a8 100644
--- a/packages/desktop-gui/src/project/error-message.jsx
+++ b/packages/desktop-gui/src/project/error-message.jsx
@@ -7,6 +7,26 @@ import { configFileFormatted } from '../lib/config-file-formatted'
 
 import Markdown from 'markdown-it'
 
+const _copyErrorDetails = (err) => {
+  let details = [
+    `**Message:** ${err.message}`,
+  ]
+
+  if (err.details) {
+    details.push(`**Details:** ${err.details}`)
+  }
+
+  if (err.title) {
+    details.unshift(`**Title:** ${err.title}`)
+  }
+
+  if (err.stack2) {
+    details.push(`**Stack trace:**\n\`\`\`\n${err.stack2}\n\`\`\``)
+  }
+
+  ipc.setClipboardText(details.join('\n\n'))
+}
+
 const md = new Markdown({
   html: true,
   linkify: true,
@@ -20,7 +40,7 @@ const ErrorDetails = observer(({ err }) => {
   if (detailsBody) {
     return (
       <pre>
-        <details>
+        <details className='details-body'>
           <summary>{detailsTitle}</summary>
           {detailsBody}
         </details>
@@ -61,7 +81,7 @@ class ErrorMessage extends Component {
         <div className='full-alert alert alert-danger error'>
           <p className='header'>
             <i className='fas fa-exclamation-triangle'></i>{' '}
-            <strong>{err.title || 'Can\'t start server'}</strong>
+            <strong>{err.title || 'An unexpected error occurred'}</strong>
           </p>
           <span className='alert-content'>
             <div ref={(node) => this.errorMessageNode = node} dangerouslySetInnerHTML={{
@@ -76,7 +96,22 @@ class ErrorMessage extends Component {
                 <p>To fix, stop the other running process or change the port in {configFileFormatted(this.props.project.configFile)}</p>
               </div>
             )}
+            {err.stack2 && (
+              <details className='stacktrace'>
+                <summary>Stack trace</summary>
+                <pre>{err.stack2}</pre>
+              </details>
+            )}
           </span>
+          <button
+            className='btn btn-default btn-sm'
+            onClick={() => {
+              _copyErrorDetails(err)
+            }}
+          >
+            <i className='fas fa-copy'></i>{' '}
+            Copy to Clipboard
+          </button>
           <button
             className='btn btn-default btn-sm'
             onClick={this.props.onTryAgain}
diff --git a/packages/desktop-gui/src/project/project-model.js b/packages/desktop-gui/src/project/project-model.js
index 0001fbaff1dc..839391dc3f4a 100644
--- a/packages/desktop-gui/src/project/project-model.js
+++ b/packages/desktop-gui/src/project/project-model.js
@@ -209,6 +209,9 @@ export default class Project {
   }
 
   @action setError (err = {}) {
+    // for some reason, the original `stack` is unavailable on `err` once it is set on the model
+    // `stack2` remains usable though, for some reason
+    err.stack2 = err.stack
     this.error = err
   }
 
diff --git a/packages/server/lib/gui/events.coffee b/packages/server/lib/gui/events.coffee
index 6dc607aac900..4c7c7b00a75b 100644
--- a/packages/server/lib/gui/events.coffee
+++ b/packages/server/lib/gui/events.coffee
@@ -1,6 +1,6 @@
 _           = require("lodash")
 ipc         = require("electron").ipcMain
-shell       = require("electron").shell
+{ shell, clipboard } = require('electron')
 debug       = require('debug')('cypress:server:events')
 pluralize   = require("pluralize")
 stripAnsi   = require("strip-ansi")
@@ -109,7 +109,10 @@ handleEvent = (options, bus, event, id, type, arg) ->
         onBrowserClose: ->
           send({browserClosed: true})
       })
-      .catch(sendErr)
+      .catch (err) =>
+        err.title ?= 'Error launching browser'
+
+        sendErr(err)
 
     when "begin:auth"
       onMessage = (msg) ->
@@ -299,6 +302,10 @@ handleEvent = (options, bus, event, id, type, arg) ->
         err.apiUrl = apiUrl
         sendErr(err)
 
+    when "set:clipboard:text"
+      clipboard.writeText(arg)
+      sendNull()
+
     else
       throw new Error("No ipc event registered for: '#{type}'")
 
diff --git a/packages/server/test/unit/gui/events_spec.coffee b/packages/server/test/unit/gui/events_spec.coffee
index b2ab00770a79..8a205120a1bf 100644
--- a/packages/server/test/unit/gui/events_spec.coffee
+++ b/packages/server/test/unit/gui/events_spec.coffee
@@ -699,3 +699,25 @@ describe "lib/gui/events", ->
           expect(err.name).to.equal("ECONNREFUSED 127.0.0.1:1234")
           expect(err.message).to.equal("ECONNREFUSED 127.0.0.1:1234")
           expect(err.apiUrl).to.equal(konfig("api_url"))
+
+    describe "launch:browser", ->
+      it "launches browser via openProject", ->
+        sinon.stub(openProject, 'launch').callsFake (browser, spec, opts) ->
+          expect(browser).to.eq('foo')
+          expect(spec).to.eq('bar')
+
+          opts.onBrowserOpen()
+          opts.onBrowserClose()
+
+          Promise.resolve()
+
+        @handleEvent("launch:browser", { browser: 'foo', spec: 'bar' }).then =>
+          expect(@send.getCall(0).args[1].data).to.include({ browserOpened: true })
+          expect(@send.getCall(1).args[1].data).to.include({ browserClosed: true })
+
+      it "wraps error titles if not set", ->
+        err = new Error('foo')
+        sinon.stub(openProject, 'launch').rejects(err)
+
+        @handleEvent("launch:browser", {}).then =>
+          expect(@send.getCall(0).args[1].__error).to.include({ message: 'foo', title: 'Error launching browser' })