Skip to content

Commit

Permalink
sync: improve error handling
Browse files Browse the repository at this point in the history
Replace:

- `showError` with `errorAndHelp`
- (`stderr.writeLine msg` then `quit 1`) with `error`

Refs: 123
Refs: 325
  • Loading branch information
ee7 committed Nov 4, 2022
1 parent ceaad10 commit 658fef6
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 54 deletions.
10 changes: 10 additions & 0 deletions src/cli.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import std/[os, parseutils, strformat, strutils, terminal]
import pkg/[cligen/parseopt3, supersnappy]

type
ConfigletError* = object of CatchableError ## Quit with exit code 1
ConfigletErrorAndHelp* = object of CatchableError ## Quit with exit code 1, and print help

func error*(s: string) =
raise newException(ConfigletError, s)

func errorAndHelp*(s: string) =
raise newException(ConfigletErrorAndHelp, s)

type
ActionKind* = enum
actNil = "nil"
Expand Down
3 changes: 3 additions & 0 deletions src/configlet.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 7 additions & 9 deletions src/sync/probspecs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 " &
Expand Down Expand Up @@ -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)
12 changes: 5 additions & 7 deletions src/sync/sync.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
29 changes: 12 additions & 17 deletions src/sync/sync_common.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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] =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions src/sync/sync_docs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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}")

Expand Down
3 changes: 1 addition & 2 deletions src/sync/sync_filepaths.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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}")

Expand Down
3 changes: 1 addition & 2 deletions src/sync/sync_metadata.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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}")

Expand Down
23 changes: 9 additions & 14 deletions src/sync/tracks.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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"):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_binary.nim
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ proc testsForSync(binaryPath: static string) =
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:
Error: 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)
Expand Down

0 comments on commit 658fef6

Please sign in to comment.