sync: improve error handling

src/cli.nim

@@ -1,6 +1,18 @@
 import std/[os, parseutils, strformat, strutils, terminal]
 import pkg/[cligen/parseopt3, supersnappy]
 
+type
+  ConfigletError* = object of CatchableError ## \
+    ## Raised to quit with exit code 1, but not print configlet usage
+  ConfigletErrorAndHelp* = object of CatchableError ## \
+    ## Raised to quit with exit code 1, and print configlet usage
+
+func error*(s: string) =
+  raise newException(ConfigletError, s)
+
+func errorAndHelp*(s: string) =
+  raise newException(ConfigletErrorAndHelp, s)
+
 type
   ActionKind* = enum
     actNil = "nil"

src/configlet.nim

@@ -31,6 +31,9 @@ proc configlet =
 proc main =
   try:
     configlet()
+  except ConfigletError:
+    let s = getCurrentExceptionMsg()
+    showError(s, writeHelp = false)
   except CatchableError:
     let msg = getCurrentExceptionMsg()
     showError(msg)

src/sync/probspecs.nim

@@ -101,8 +101,8 @@ proc getNameOfRemote*(probSpecsDir: ProbSpecsDir;
     discard line.scanf("$s$w$s$+fetch)$.", remoteName, remoteUrl)
     if remoteUrl.contains(host) and remoteUrl.contains(location):
       return remoteName
-  showError(&"there is no remote that points to '{location}' at '{host}' in " &
-            &"the cached problem-specifications directory: '{probSpecsDir}'")
+  errorAndHelp &"there is no remote that points to '{location}' at '{host}' in " &
+               &"the cached problem-specifications directory: '{probSpecsDir}'"
 
 func isOffline(conf: Conf): bool =
   (conf.action.kind == actSync and conf.action.offline) or
@@ -124,8 +124,8 @@ proc validate(probSpecsDir: ProbSpecsDir, conf: Conf) =
                                  &"git repository: '{probSpecsDir}'")
 
     if rootCommitRef != "8ba81069dab8e96a53630f3e51446487b6ec9212\n":
-      showError("the git repo at the cached problem-specifications location " &
-                &"has an unexpected initial commit: '{probSpecsDir}'")
+      errorAndHelp "the git repo at the cached problem-specifications location " &
+                   &"has an unexpected initial commit: '{probSpecsDir}'"
 
     # Exit if the working directory is not clean (allowing untracked files).
     discard gitCheck(0, ["diff-index", "--quiet", "HEAD"], "the cached " &
@@ -189,19 +189,17 @@ proc init*(T: typedesc[ProbSpecsDir], conf: Conf): T =
     validate(result, conf)
   elif isOffline(conf):
     let msg = fmt"""
-      Error: --offline was passed, but there is no cached 'problem-specifications' repo at:
+      --offline was passed, but there is no cached 'problem-specifications' repo at:
         '{result}'
       Please run once without --offline to clone 'problem-specifications' to that location.
 
       If you currently have no (or limited) network connectivity, but you do have a local
       'problem-specifications' elsewhere, you can copy it to the above location and then
       use it with --offline.""".unindent()
-    stderr.writeLine msg
-    quit 1
+    error msg
   else:
     try:
       createDir result.parentDir()
     except IOError, OSError:
-      stderr.writeLine &"Error: {getCurrentExceptionMsg()}"
-      quit 1
+      error getCurrentExceptionMsg()
     cloneExercismRepo("problem-specifications", result.string, shallow = false)

src/sync/sync.nim

@@ -21,7 +21,7 @@ proc validate(conf: Conf) =
 
         If no syncing scope option is provided, configlet uses the full syncing scope.
         If '--tests' is provided without an argument, configlet uses the 'choose' mode.""".dedent(8)
-      showError(msg)
+      errorAndHelp msg
     if not conf.action.yes and not isatty(stdin):
       let intersection = conf.action.scope * {skDocs, skFilepaths, skMetadata}
       if intersection.len > 0:
@@ -37,7 +37,7 @@ proc validate(conf: Conf) =
             that configlet performs no changes
           - or run the same command in an interactive terminal, to update by answering
             prompts""".dedent(10)
-        showError(msg)
+        errorAndHelp msg
 
 type
   TrackExerciseSlugs* = object
@@ -74,11 +74,9 @@ proc getSlugs*(exercises: Exercises, conf: Conf,
       result.`concept`.setLen 0
       result.practice = @[userExercise]
     else:
-      let msg = &"The `-e, --exercise` option was used to specify an " &
-                &"exercise slug, but `{userExercise}` is not an slug in the " &
-                &"track config:\n{trackConfigPath}"
-      stderr.writeLine msg
-      quit 1
+      error "The `-e, --exercise` option was used to specify an exercise " &
+            &"slug, but `{userExercise}` is not an slug in the track config:" &
+            &"\n{trackConfigPath}"
 
 proc syncImpl(conf: Conf): set[SyncKind] =
   ## Checks the data specified in `conf.action.scope`, and updates them if

src/sync/sync_common.nim

@@ -20,10 +20,9 @@ proc postHook*(e: ConceptExercise | PracticeExercise) =
   ## string.
   let s = e.slug.string
   if not isKebabCase(s):
-    let msg = "Error: the track `config.json` file contains " &
-              &"an exercise slug of \"{s}\", which is not a kebab-case string"
-    stderr.writeLine msg
-    quit 1
+    let msg = "the track `config.json` file contains an exercise slug of " &
+              &"\"{s}\", which is not a kebab-case string"
+    error msg
 
 func getSlugs*(e: seq[ConceptExercise] | seq[PracticeExercise],
                withDeprecated = true): seq[Slug] =
@@ -107,24 +106,22 @@ func renameHook*(f: var (ConceptExerciseFiles | PracticeExerciseFiles); key: str
 
 proc parseFile*(path: string, T: typedesc): T =
   ## Parses the JSON file at `path` into `T`.
-  let contents =
+  let contents = block:
+    var res = ""
     try:
-      readFile(path)
+      res = readFile(path)
     except IOError:
-      let msg = getCurrentExceptionMsg()
-      stderr.writeLine &"Error: {msg}"
-      quit 1
+      error getCurrentExceptionMsg()
+    res
   if contents.len > 0:
     try:
-      contents.fromJson(T)
+      result = contents.fromJson(T)
     except jsony.JsonError:
       let jsonyMsg = getCurrentExceptionMsg()
       let details = tidyJsonyMessage(jsonyMsg, contents)
-      let msg = &"JSON parsing error:\n{path}{details}"
-      stderr.writeLine msg
-      quit 1
+      error &"JSON parsing error:\n{path}{details}"
   else:
-    T()
+    result = T()
 
 func addNewlineAndIndent(s: var string, indentLevel: int) =
   ## Appends a newline and spaces (given by `indentLevel` multiplied by 2) to
@@ -269,9 +266,7 @@ proc addObject(s: var string; key: string; val: JsonNode; indentLevel = 1) =
       else:
         s.add c
   else:
-    stderr.writeLine &"The value of a `{key}` key is not a JSON object:"
-    stderr.writeLine val.pretty()
-    quit 1
+    error &"The value of a `{key}` key is not a JSON object:\n{val.pretty()}"
 
 func keyOrderForSync(originalKeyOrder: seq[ExerciseConfigKey]): seq[ExerciseConfigKey] =
   if originalKeyOrder.len == 0:

src/sync/sync_docs.nim

@@ -122,8 +122,7 @@ proc write(pairsToWrite: seq[PathAndContents]) =
       createDir path.parentDir()
       writeFile(path, pathAndContents.contents)
     else:
-      stderr.writeLine &"Unexpected path before writing: {path}"
-      quit 1
+      error &"Unexpected path before writing: {path}"
   let s = if pairsToWrite.len > 1: "s" else: ""
   logNormal(&"Updated the docs for {pairsToWrite.len} Practice Exercise{s}")
 

src/sync/sync_filepaths.nim

@@ -147,8 +147,7 @@ proc write(configPairs: seq[PathAndUpdatedExerciseConfig]) =
           configPair.exerciseConfig.p.pretty(prettyMode = pmSync)
       writeFile(path, contents)
     else:
-      stderr.writeLine &"Unexpected path before writing: {path}"
-      quit 1
+      error &"Unexpected path before writing: {path}"
   let s = if configPairs.len > 1: "s" else: ""
   logNormal(&"Updated the filepaths for {configPairs.len} exercise{s}")
 

src/sync/sync_metadata.nim

@@ -100,8 +100,7 @@ proc write(configPairs: seq[PathAndUpdatedConfig]) =
       createDir path.parentDir()
       writeFile(path, updatedJson)
     else:
-      stderr.writeLine &"Unexpected path before writing: {path}"
-      quit 1
+      error &"Unexpected path before writing: {path}"
   let s = if configPairs.len > 1: "s" else: ""
   logNormal(&"Updated the metadata for {configPairs.len} Practice Exercise{s}")
 

src/sync/tracks.nim

@@ -48,13 +48,9 @@ proc getPracticeExerciseSlugs(trackDir: TrackDir,
               let slug = exercise["slug"].getStr()
               result.add PracticeExerciseSlug(slug)
     else:
-      stderr.writeLine "Error: file does not have an `exercises` key:\n" &
-                       configFile
-      quit(1)
+      error &"file does not have an `exercises` key:\n{configFile}"
   else:
-    stderr.writeLine "Error: file does not exist:\n" & configFile
-    quit(1)
-
+    error &"file does not exist:\n{configFile}"
   sort result
 
 func init(T: typedesc[PracticeExerciseTests]): T =
@@ -68,21 +64,20 @@ proc init(T: typedesc[PracticeExerciseTests], testsPath: string): T =
   ## included and excluded test case UUIDs.
   result = PracticeExerciseTests.init()
   if fileExists(testsPath):
-    let tests =
+    let tests = block:
+      var res = TomlValueRef()
       try:
-        parsetoml.parseFile(testsPath)
+        res = parsetoml.parseFile(testsPath)
       except TomlError: # Note that `TomlError` inherits from `Defect`.
-        stderr.writeLine fmt"""
+        let msg = fmt"""
 
-          Error: A 'tests.toml' file contains invalid TOML:
+          a 'tests.toml' file contains invalid TOML:
           {getCurrentExceptionMsg()}
 
           The expected 'tests.toml' format is documented in
           https://exercism.org/docs/building/configlet/sync#h-tests""".unindent()
-        quit 1
-      except CatchableError:
-        stderr.writeLine "Error: " & getCurrentExceptionMsg()
-        quit 1
+        error msg
+      res
 
     for uuid, val in tests.getTable():
       if val.hasKey("include"):

tests/test_binary.nim

@@ -140,18 +140,24 @@ proc testsForSync(binaryPath: static string) =
   suite "sync, when the track `config.json` file is not found (prints the expected output, and exits with 1)":
     test "-t foo":
       const expectedOutput = fmt"""
-        Error: cannot open: my_missing_directory{DirSep}config.json
+        cannot open: my_missing_directory{DirSep}config.json
       """.unindent()
       let cmd = &"{binaryPath} -t my_missing_directory sync -o"
-      execAndCheck(1, cmd, expectedOutput)
+      let (outp, exitCode) = execCmdEx(cmd)
+      check:
+        outp.contains(expectedOutput)
+        exitCode == 1
 
   suite "sync, for an exercise that does not exist (prints the expected output, and exits with 1)":
     test "-e foo":
       const expectedOutput = fmt"""
         The `-e, --exercise` option was used to specify an exercise slug, but `foo` is not an slug in the track config:
         {trackDir / "config.json"}
       """.unindent()
-      execAndCheck(1, &"{syncOffline} -e foo --docs", expectedOutput)
+      let (outp, exitCode) = execCmdEx(&"{syncOffline} -e foo --docs")
+      check:
+        outp.contains(expectedOutput)
+        exitCode == 1
 
   suite "sync, without --update, checking parseopt3 patch (--tests -e bob is parsed correctly)":
     # With an unpatched cligen/parseopt3, we can only write `--tests` without a