From 70e5916a288dd7dad05244dd337bbf4a7b72fc05 Mon Sep 17 00:00:00 2001 From: Brian Mann Date: Sun, 4 Mar 2018 01:17:06 -0500 Subject: [PATCH] server: fixes #1192 pass reporter errors along and provide stack if module doesn't exist (#1411) --- .../__snapshots__/reporters_spec.coffee | 50 +++++++++++++++++++ packages/server/lib/errors.coffee | 10 ++-- packages/server/lib/project.coffee | 23 +++++++-- packages/server/lib/reporter.coffee | 33 ++++++------ .../server/test/e2e/reporters_spec.coffee | 9 ++++ .../fixtures/projects/e2e/reporters/throws.js | 1 + 6 files changed, 101 insertions(+), 25 deletions(-) create mode 100644 packages/server/test/support/fixtures/projects/e2e/reporters/throws.js diff --git a/packages/server/__snapshots__/reporters_spec.coffee b/packages/server/__snapshots__/reporters_spec.coffee index 08d07d3fb230..cb41f03bf205 100644 --- a/packages/server/__snapshots__/reporters_spec.coffee +++ b/packages/server/__snapshots__/reporters_spec.coffee @@ -5,6 +5,10 @@ We searched for the reporter in these paths: - /foo/bar/.projects/e2e/module-does-not-exist - /foo/bar/.projects/e2e/node_modules/module-does-not-exist +The error we received was: + +Cannot find module '/foo/bar/.projects/e2e/node_modules/module-does-not-exist' + Learn more at stack trace line ` @@ -347,3 +351,49 @@ Because this error occurred during a 'before each' hook we are skipping the rema ` +exports['e2e reporters reports error when thrown from reporter 1'] = `Could not load reporter by name: reporters/throws.js + +We searched for the reporter in these paths: + +- /foo/bar/.projects/e2e/reporters/throws.js +- /foo/bar/.projects/e2e/node_modules/reporters/throws.js + +The error we received was: + +Error: this reporter threw an error + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + at stack trace line + + +Learn more at stack trace line +` + diff --git a/packages/server/lib/errors.coffee b/packages/server/lib/errors.coffee index 92cdc4965d3d..25dd5a42e1d5 100644 --- a/packages/server/lib/errors.coffee +++ b/packages/server/lib/errors.coffee @@ -11,7 +11,7 @@ listPaths = (paths) -> API = { # forms well-formatted user-friendly error for most common # errors Cypress can encounter - getMsgByType: (type, arg1, arg2) -> + getMsgByType: (type, arg1 = {}, arg2 = {}) -> switch type when "CANNOT_TRASH_ASSETS" """ @@ -326,11 +326,15 @@ API = { """ when "INVALID_REPORTER_NAME" """ - Could not load reporter by name: #{chalk.yellow(arg1)} + Could not load reporter by name: #{chalk.yellow(arg1.name)} We searched for the reporter in these paths: - #{listPaths(arg2).join("\n")} + #{listPaths(arg1.paths).join("\n")} + + The error we received was: + + #{chalk.yellow(arg1.error)} Learn more at https://on.cypress.io/reporters """ diff --git a/packages/server/lib/project.coffee b/packages/server/lib/project.coffee index 48cec002e678..714373d37319 100644 --- a/packages/server/lib/project.coffee +++ b/packages/server/lib/project.coffee @@ -221,14 +221,27 @@ class Project extends EE watchSettingsAndStartWebsockets: (options = {}, cfg = {}) -> @watchSettings(options.onSettingsChanged) + { reporter, projectRoot } = cfg + ## if we've passed down reporter ## then record these via mocha reporter if cfg.report - if not Reporter.isValidReporterName(cfg.reporter, cfg.projectRoot) - paths = Reporter.getSearchPathsForReporter(cfg.reporter, cfg.projectRoot) - errors.throw("INVALID_REPORTER_NAME", cfg.reporter, paths) - - reporter = Reporter.create(cfg.reporter, cfg.reporterOptions, cfg.projectRoot) + try + Reporter.loadReporter(reporter, projectRoot) + catch err + paths = Reporter.getSearchPathsForReporter(reporter, projectRoot) + + ## only include the message if this is the standard MODULE_NOT_FOUND + ## else include the whole stack + errorMsg = if err.code is "MODULE_NOT_FOUND" then err.message else err.stack + + errors.throw("INVALID_REPORTER_NAME", { + paths + error: errorMsg + name: reporter + }) + + reporter = Reporter.create(reporter, cfg.reporterOptions, projectRoot) @automation = Automation.create(cfg.namespace, cfg.socketIoCookie, cfg.screenshotsFolder) diff --git a/packages/server/lib/reporter.coffee b/packages/server/lib/reporter.coffee index ff2bd9c09d42..a42bc9d378dd 100644 --- a/packages/server/lib/reporter.coffee +++ b/packages/server/lib/reporter.coffee @@ -206,7 +206,8 @@ class Reporter new Reporter(reporterName, reporterOptions, projectRoot) @loadReporter = (reporterName, projectRoot) -> - debug("loading reporter #{reporterName}") + debug("trying to load reporter:", reporterName) + if r = reporters[reporterName] debug("#{reporterName} is built-in reporter") return require(r) @@ -219,22 +220,28 @@ class Reporter ## that is local (./custom-reporter.js) ## or one installed by the user through npm try + p = path.resolve(projectRoot, reporterName) + ## try local - debug("loading local reporter by name #{reporterName}") + debug("trying to require local reporter with path:", p) ## using path.resolve() here so we can just pass an ## absolute path as the reporterName which avoids ## joining projectRoot unnecessarily - return require(path.resolve(projectRoot, reporterName)) + return require(p) catch err + if err.code isnt "MODULE_NOT_FOUND" + ## bail early if the error wasn't MODULE_NOT_FOUND + ## because that means theres something actually wrong + ## with the found reporter + throw err + + p = path.resolve(projectRoot, "node_modules", reporterName) + ## try npm. if this fails, we're out of options, so let it throw - debug("loading NPM reporter module #{reporterName} from #{projectRoot}") + debug("trying to require local reporter with path:", p) - try - return require(path.resolve(projectRoot, "node_modules", reporterName)) - catch err - msg = "Could not find reporter module #{reporterName} relative to #{projectRoot}" - throw new Error(msg) + return require(p) @getSearchPathsForReporter = (reporterName, projectRoot) -> _.uniq([ @@ -242,12 +249,4 @@ class Reporter path.resolve(projectRoot, "node_modules", reporterName) ]) - @isValidReporterName = (reporterName, projectRoot) -> - try - Reporter.loadReporter(reporterName, projectRoot) - debug("reporter #{reporterName} is valid name") - true - catch - false - module.exports = Reporter diff --git a/packages/server/test/e2e/reporters_spec.coffee b/packages/server/test/e2e/reporters_spec.coffee index da1c26a12895..9e21a01d67ea 100644 --- a/packages/server/test/e2e/reporters_spec.coffee +++ b/packages/server/test/e2e/reporters_spec.coffee @@ -27,6 +27,15 @@ describe "e2e reporters", -> reporter: "module-does-not-exist" }) + ## https://github.com/cypress-io/cypress/issues/1192 + it "reports error when thrown from reporter", -> + e2e.exec(@, { + spec: "simple_passing_spec.coffee" + snapshot: true + expectedExitCode: 1 + reporter: "reporters/throws.js" + }) + it "supports junit reporter and reporter options", -> e2e.exec(@, { spec: "simple_passing_spec.coffee" diff --git a/packages/server/test/support/fixtures/projects/e2e/reporters/throws.js b/packages/server/test/support/fixtures/projects/e2e/reporters/throws.js new file mode 100644 index 000000000000..cf6bd67a0fb9 --- /dev/null +++ b/packages/server/test/support/fixtures/projects/e2e/reporters/throws.js @@ -0,0 +1 @@ +throw new Error('this reporter threw an error')