Skip to content

Commit

Permalink
feat(errors): add more verbose InternalCliError class with stack trace
Browse files Browse the repository at this point in the history
  • Loading branch information
ctjlewis committed Jan 24, 2021
1 parent 6a633c1 commit 2e06694
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 36 deletions.
57 changes: 32 additions & 25 deletions src/detectors/utils/jsdetect.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
*/
const { existsSync, readFileSync } = require('fs')

const { InternalCliError } = require('../../utils/error')
const { NETLIFYDEVWARN } = require('../../utils/logo')

let pkgJSON = null
let yarnExists = false
let warnedAboutEmptyScript = false
const { NETLIFYDEVWARN } = require('../../utils/logo')

/** hold package.json in a singleton so we dont do expensive parsing repeatedly */
const getPkgJSON = function () {
Expand All @@ -28,9 +30,7 @@ const getYarnOrNPMCommand = function () {

/**
* real utiltiies are down here
*
*/

const hasRequiredDeps = function (requiredDepArray) {
const { dependencies, devDependencies } = getPkgJSON()
for (const depName of requiredDepArray) {
Expand All @@ -53,9 +53,9 @@ const hasRequiredFiles = function (filenameArr) {

// preferredScriptsArr is in decreasing order of preference
const scanScripts = function ({ preferredScriptsArr, preferredCommand }) {
const { scripts } = getPkgJSON()
const packageJsonScripts = getPkgJSON().scripts

if (!scripts && !warnedAboutEmptyScript) {
if (!packageJsonScripts && !warnedAboutEmptyScript) {
console.log(`${NETLIFYDEVWARN} You have a package.json without any npm scripts.`)
console.log(
`${NETLIFYDEVWARN} Netlify Dev's detector system works best with a script, or you can specify a command to run in the netlify.toml [dev] block `,
Expand All @@ -65,26 +65,33 @@ const scanScripts = function ({ preferredScriptsArr, preferredCommand }) {
// not going to match any scripts anyway
return []
}
//
//
// NOTE: we return an array of arrays (args)
// because we may want to supply extra args in some setups
//
// e.g. ['eleventy', '--serve', '--watch']
//
// array will in future be sorted by likelihood of what we want
//
//
// this is very simplistic logic, we can offer far more intelligent logic later
// eg make a dependency tree of npm scripts and offer the parentest node first
return Object.entries(scripts)
.filter(
([scriptName, scriptCommand]) =>
(preferredScriptsArr.includes(scriptName) || scriptCommand.includes(preferredCommand)) &&
// prevent netlify dev calling netlify dev
!scriptCommand.includes('netlify dev'),
)
.map(([scriptName]) => [scriptName])
/**
* NOTE: we return an array of arrays (args) because we may want to supply
* extra args in some setups, e.g.
*
* ['eleventy', '--serve', '--watch']
*
* array will be sorted by likelihood of what we want in the future. this is
* very simplistic logic, we can offer far more intelligent logic later, e.g.
* make a dependency tree of npm scripts and offer the parentest node first
*/
const matchedScripts = []
for (const [scriptName, scriptCommand] of Object.entries(packageJsonScripts)) {
/**
* Throw if trying to call Netlify dev from within Netlify dev. Include
* detailed information about the CLI setup in the error text.
*/
if (scriptCommand.includes('netlify dev')) {
throw new InternalCliError('Cannot call `netlify dev` inside `netlify dev`.', { packageJsonScripts })
}
/**
* Otherwise, push the match.
*/
if (preferredScriptsArr.includes(scriptName) || scriptCommand.includes(preferredCommand)) {
matchedScripts.push([scriptName])
}
}
return matchedScripts
}

module.exports = {
Expand Down
30 changes: 20 additions & 10 deletions src/utils/detect-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const getPort = require('get-port')
const inquirer = require('inquirer')
const inquirerAutocompletePrompt = require('inquirer-autocomplete-prompt')

const { InternalCliError } = require('./error')
const { NETLIFYDEVLOG, NETLIFYDEVWARN } = require('./logo')

const serverSettings = async (devConfig, flags, projectDir, log) => {
Expand Down Expand Up @@ -222,20 +223,29 @@ const loadDetector = function (detectorName) {
}
}

/**
* Get the default args from an array of possible args.
*
* @param {Array<Array>?} possibleArgsArrs
* @returns {Array}
*/
const chooseDefaultArgs = function (possibleArgsArrs) {
// vast majority of projects will only have one matching detector
// just pick the first one
/**
* Select first set of possible args.
*/
const [args] = possibleArgsArrs
if (!args) {
const { scripts } = JSON.parse(fs.readFileSync('package.json', { encoding: 'utf8' }))
const err = new Error(
'Empty args assigned, this is an internal Netlify Dev bug, please report your settings and scripts so we can improve',
)
err.scripts = scripts
err.possibleArgsArrs = possibleArgsArrs
throw err
/**
* Load scripts from package.json (if it exists) to display them in the
* error. Initialize `scripts` to `null` so it is not omitted by
* JSON.stringify() in the case it isn't reassigned.
*/
let packageJsonScripts = null
if (fs.existsSync('package.json')) {
packageJsonScripts = JSON.parse(fs.readFileSync('package.json')).scripts
}
throw new InternalCliError('No possible args found.', { packageJsonScripts, possibleArgsArrs })
}

return args
}

Expand Down
2 changes: 1 addition & 1 deletion src/utils/detect-server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ test('serverSettings: no config', async (t) => {
t.is(settings.noCmd, true)
})

test('chooseDefaultArgs', (t) => {
test('chooseDefaultArgs: select first from possibleArgsArrs', (t) => {
const possibleArgsArrs = [['run', 'dev'], ['run develop']]
const args = chooseDefaultArgs(possibleArgsArrs)
t.deepEqual(args, possibleArgsArrs[0])
Expand Down
27 changes: 27 additions & 0 deletions src/utils/error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* An unrecoverable internal CLI error which should be reported.
*/
class InternalCliError extends Error {
/**
* Log a stack trace and the given context object for opening an issue, then
* throw.
*
* @param {string!} message
* @param {Object!} context
*/
constructor(message, context) {
super(message)
this.name = 'InternalCliError'

console.trace(
`INTERNAL CLI ERROR. ${message}\n` +
'Please open an issue at https://github.com/netlify/cli/issues/new ' +
'and include the following information:' +
`\n${JSON.stringify(context, null, 2)}\n`,
)
}
}

module.exports = {
InternalCliError,
}

0 comments on commit 2e06694

Please sign in to comment.