Merge pull request #692 from Financial-Times/improve-errors

Improve Tool Kit error handling

core/cli/src/install.ts

@@ -1,5 +1,7 @@
+import * as path from 'path'
 import type { z } from 'zod'
 import { ToolKitError } from '@dotcom-tool-kit/error'
+import { styles } from '@dotcom-tool-kit/logger'
 import { OptionKey, setOptions } from '@dotcom-tool-kit/options'
 import groupBy from 'lodash/groupBy'
 import type { Logger } from 'winston'
@@ -65,9 +67,15 @@ export const loadHookInstallations = async (
     if (conflicts.length) {
       return invalid<[]>(
         conflicts.map(
-          // TODO:20240429:IM format a more helpful error message here
           (conflict) =>
-            `conflicts between ${conflict.conflicting.map((installation) => installation.forHook).join(', ')}`
+            `hooks installation conflicts between ${conflict.conflicting
+              .map(
+                (installation, i, array) =>
+                  `${i === array.length - 1 ? 'and ' : ''}the ${styles.hook(
+                    installation.forHook
+                  )} hook from ${styles.filepath(path.relative('', installation.plugin.root))}`
+              )
+              .join(', ')}`
         )
       )
     }
@@ -89,7 +97,9 @@ export async function checkInstall(logger: Logger, config: ValidConfig): Promise
     return
   }
 
-  const hooks = (await loadHookInstallations(logger, config)).unwrap('hooks are invalid')
+  const hooks = (await loadHookInstallations(logger, config)).unwrap(
+    'hooks were found to be invalid when checking install'
+  )
 
   const uninstalledHooks = await asyncFilter(hooks, async (hook) => {
     return !(await hook.isInstalled())
@@ -116,7 +126,9 @@ export default async function installHooks(logger: Logger): Promise<ValidConfig>
   await runInit(logger, config)
 
   const errors: Error[] = []
-  const hooks = (await loadHookInstallations(logger, config)).unwrap('hooks are invalid')
+  const hooks = (await loadHookInstallations(logger, config)).unwrap(
+    'hooks were found to be invalid when installing'
+  )
 
   // group hooks without an installGroup separately so that their check()
   // method runs independently

core/cli/src/messages.ts

@@ -120,7 +120,9 @@ They could be misspelt, or defined by a Tool Kit plugin that isn't installed in
 
 ${
   definedPlugins.length > 0
-    ? `Plugins that are defined and can have options set are: ${definedPlugins.map(s.plugin).join(', ')}`
+    ? `Plugins that are defined and can have options set are: ${definedPlugins
+        .map((plugin) => s.plugin(plugin))
+        .join(', ')}`
     : `There are no plugins installed currently. You'll need to install some plugins before options can be set.`
 }.
 `
@@ -136,7 +138,9 @@ They could be misspelt, or defined by a Tool Kit plugin that isn't installed in
 
 ${
   definedTasks.length > 0
-    ? `Task that are defined and can have options set are: ${definedTasks.map(s.task).join(', ')}`
+    ? `Task that are defined and can have options set are: ${definedTasks
+        .map((task) => s.task(task))
+        .join(', ')}`
     : `You don't have currently any plugins installed that provide tasks. You'll need to install some plugins before options can be set.`
 }.
 `
@@ -166,7 +170,7 @@ ${missingTasks.map(formatMissingTask).join('\n')}
 
 They could be misspelt, or defined by a Tool Kit plugin that isn't used by this app.
 
-Available tasks are: ${tasks.map(s.task).join(', ')}.
+Available tasks are: ${tasks.map((task) => s.task(task)).join(', ')}.
 `
 
 export function formatPluginTree(plugin: Plugin): string[] {

core/cli/src/plugin.ts

@@ -53,11 +53,13 @@ export async function loadPlugin(
 
   if (!isAppRoot && plugin.rcFile.version !== CURRENT_RC_FILE_VERSION) {
     return invalid([
-      `plugin ${s.plugin(id)} has a v${s.code((plugin.rcFile.version ?? 1).toString())} ${s.code(
+      `plugin ${s.plugin(id)} has a ${s.code('v' + (plugin.rcFile.version ?? 1).toString())} ${s.filepath(
         '.toolkitrc.yml'
-      )}, but this version of Tool Kit can only load v${s.code(
-        CURRENT_RC_FILE_VERSION.toString()
-      )}. please update this plugin.`
+      )}, but this version of Tool Kit can only load ${s.code(
+        'v' + CURRENT_RC_FILE_VERSION.toString()
+      )} configs. please update this plugin. if it's your own custom plugin you can do this be adding ${s.code(
+        'version: 2'
+      )} to the top of its ${s.filepath('.toolkitrc.yml')}.`
     ])
   }
 

core/cli/src/rc-file.ts

@@ -50,9 +50,10 @@ const toolKitIfDefined = {
 } satisfies YAML.ScalarTag
 
 export async function loadToolKitRC(logger: Logger, root: string): Promise<RCFile> {
+  const configPath = path.join(root, '.toolkitrc.yml')
   let rawConfig: string
   try {
-    rawConfig = await fs.readFile(path.join(root, '.toolkitrc.yml'), 'utf8')
+    rawConfig = await fs.readFile(configPath, 'utf8')
   } catch (err) {
     if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
       return emptyConfig
@@ -77,7 +78,7 @@ export async function loadToolKitRC(logger: Logger, root: string): Promise<RCFil
   const config: RawRCFile = configDoc.toJS() ?? {}
 
   // if a toolkitrc contains a non-empty options field, but not options.{plugins,tasks,hooks},
-  // assume it's an old-style, plugins-only options field.
+  // assume it's an old-style, (hopefully) plugins-only options field.
   // TODO:KB:20240410 remove this legacy options field handling in a future major version
   if (
     config.options &&
@@ -89,11 +90,15 @@ export async function loadToolKitRC(logger: Logger, root: string): Promise<RCFil
     }
 
     logger.warn(
-      `plugin at ${s.filepath(path.dirname(root))} has an ${s.code(
+      `config at ${s.filepath(configPath)} has an ${s.code(
         'options'
-      )} field that only contains plugin options. these options should be moved to ${s.code(
+      )} field, but it isn't specified whether the options are for ${s.plugin('plugins')}, ${s.task(
+        'tasks'
+      )}, or ${s.hook(
+        'hooks'
+      )}. we'll assume these options are for plugins (which may be incorrect!), but this is deprecated and these options should be moved to ${s.code(
         'options.plugins'
-      )}.`
+      )}, ${s.code('options.tasks')}, and ${s.code('options.hooks')} as appropriate.\n`
     )
   }
 

lib/schemas/package.json

@@ -16,5 +16,8 @@
   },
   "keywords": [],
   "author": "",
-  "license": "ISC"
+  "license": "ISC",
+  "dependencies": {
+    "@dotcom-tool-kit/logger": "^4.0.0"
+  }
 }

lib/schemas/src/hooks/circleci.ts

@@ -1,5 +1,8 @@
 import { z } from 'zod'
 
+export const CircleCiCustom = z.record(z.unknown())
+export type CircleCiCustom = z.infer<typeof CircleCiCustom>
+
 export const CircleCiExecutor = z.object({
   name: z.string(),
   image: z.string()
@@ -17,19 +20,19 @@ export const CircleCiWorkflowJob = z.object({
   requires: z.array(z.string()),
   splitIntoMatrix: z.boolean().optional(),
   runOnRelease: z.boolean().default(true),
-  custom: z.unknown().optional()
+  custom: CircleCiCustom.optional()
 })
 export type CircleCiWorkflowJob = z.infer<typeof CircleCiWorkflowJob>
 
 export const CircleCiWorkflow = z.object({
   name: z.string(),
   jobs: z.array(CircleCiWorkflowJob),
   runOnRelease: z.boolean().optional(),
-  custom: z.unknown().optional()
+  custom: CircleCiCustom.optional()
 })
 export type CircleCiWorkflow = z.infer<typeof CircleCiWorkflow>
 
-export const CircleCiCustomConfig = z.record(z.unknown())
+export const CircleCiCustomConfig = CircleCiCustom
 export type CircleCiCustomConfig = z.infer<typeof CircleCiCustomConfig>
 
 export const CircleCiSchema = z.object({

lib/schemas/tsconfig.json

@@ -3,5 +3,10 @@
   "compilerOptions": {
     "outDir": "lib",
     "rootDir": "src"
-  }
+  },
+  "references": [
+    {
+      "path": "../logger"
+    }
+  ]
 }