From 356d56a5f73c0c481117c33a9fb45c2acbeb2617 Mon Sep 17 00:00:00 2001 From: Carl Jackson Date: Tue, 22 Apr 2014 11:12:23 +0200 Subject: [PATCH] Return typed errors in param Instead of returning awkward untyped error strings, return real error structs. This will allow users of the library to extract interesting semantic meaning from our errors, and is just all around less awful than what we had before. --- param/error_helpers.go | 9 ---- param/errors.go | 112 +++++++++++++++++++++++++++++++++++++++++ param/parse.go | 95 +++++++++++++++++++++++----------- param/struct.go | 8 ++- 4 files changed, 184 insertions(+), 40 deletions(-) create mode 100644 param/errors.go diff --git a/param/error_helpers.go b/param/error_helpers.go index 4588e84..9477d3a 100644 --- a/param/error_helpers.go +++ b/param/error_helpers.go @@ -6,21 +6,12 @@ import ( "log" ) -// TODO: someday it might be nice to throw typed errors instead of weird strings - // Testing log.Fatal in tests is... not a thing. Allow tests to stub it out. var pebkacTesting bool const errPrefix = "param/parse: " const yourFault = " This is a bug in your use of the param library." -// Panic with a formatted error. The param library uses panics to quickly unwind -// the call stack and return a user error -func perr(format string, a ...interface{}) { - err := errors.New(errPrefix + fmt.Sprintf(format, a...)) - panic(err) -} - // Problem exists between keyboard and chair. This function is used in cases of // programmer error, i.e. an inappropriate use of the param library, to // immediately force the program to halt with a hopefully helpful error message. diff --git a/param/errors.go b/param/errors.go new file mode 100644 index 0000000..64011e7 --- /dev/null +++ b/param/errors.go @@ -0,0 +1,112 @@ +package param + +import ( + "fmt" + "reflect" +) + +// TypeError is an error type returned when param has difficulty deserializing a +// parameter value. +type TypeError struct { + // The key that was in error. + Key string + // The type that was expected. + Type reflect.Type + // The underlying error produced as part of the deserialization process, + // if one exists. + Err error +} + +func (t TypeError) Error() string { + return fmt.Sprintf("param: error parsing key %q as %v: %v", t.Key, t.Type, + t.Err) +} + +// SingletonError is an error type returned when a parameter is passed multiple +// times when only a single value is expected. For example, for a struct with +// integer field "foo", "foo=1&foo=2" will return a SingletonError with key +// "foo". +type SingletonError struct { + // The key that was in error. + Key string + // The type that was expected for that key. + Type reflect.Type + // The list of values that were provided for that key. + Values []string +} + +func (s SingletonError) Error() string { + return fmt.Sprintf("param: error parsing key %q: expected single "+ + "value but was given %d: %v", s.Key, len(s.Values), s.Values) +} + +// NestingError is an error type returned when a key is nested when the target +// type does not support nesting of the given type. For example, deserializing +// the parameter key "anint[foo]" into a struct that defines an integer param +// "anint" will produce a NestingError with key "anint" and nesting "[foo]". +type NestingError struct { + // The portion of the key that was correctly parsed into a value. + Key string + // The type of the key that was invalidly nested on. + Type reflect.Type + // The portion of the key that could not be parsed due to invalid + // nesting. + Nesting string +} + +func (n NestingError) Error() string { + return fmt.Sprintf("param: error parsing key %q: invalid nesting "+ + "%q on %s key %q", n.Key+n.Nesting, n.Nesting, n.Type, n.Key) +} + +// SyntaxErrorSubtype describes what sort of syntax error was encountered. +type SyntaxErrorSubtype int + +const ( + MissingOpeningBrace SyntaxErrorSubtype = iota + 1 + MissingClosingBrace +) + +// SyntaxError is an error type returned when a key is incorrectly formatted. +type SyntaxError struct { + // The key for which there was a syntax error. + Key string + // The subtype of the syntax error, which describes what sort of error + // was encountered. + Subtype SyntaxErrorSubtype + // The part of the key (generally the suffix) that was in error. + ErrorPart string +} + +func (s SyntaxError) Error() string { + prefix := fmt.Sprintf("param: syntax error while parsing key %q: ", + s.Key) + + switch s.Subtype { + case MissingOpeningBrace: + return prefix + fmt.Sprintf("expected opening brace, got %q", + s.ErrorPart) + case MissingClosingBrace: + return prefix + fmt.Sprintf("expected closing brace in %q", + s.ErrorPart) + default: + panic("switch is not exhaustive!") + } +} + +// KeyError is an error type returned when an unknown field is set on a struct. +type KeyError struct { + // The full key that was in error. + FullKey string + // The key of the struct that did not have the given field. + Key string + // The type of the struct that did not have the given field. + Type reflect.Type + // The name of the field which was not present. + Field string +} + +func (k KeyError) Error() string { + return fmt.Sprintf("param: error parsing key %q: unknown field %q on "+ + "struct %q of type %v", k.FullKey, k.Field, k.Key, k.Type) +} diff --git a/param/parse.go b/param/parse.go index 7239239..1213d8c 100644 --- a/param/parse.go +++ b/param/parse.go @@ -59,39 +59,60 @@ func kpath(key, keytail string) string { // Helper for validating that a value has been passed exactly once, and that the // user is not attempting to nest on the key. -func primitive(tipe, key, keytail string, values []string) { +func primitive(key, keytail string, tipe reflect.Type, values []string) { if keytail != "" { - perr("expected %s for key %q, got nested value", tipe, - kpath(key, keytail)) + panic(NestingError{ + Key: kpath(key, keytail), + Type: tipe, + Nesting: keytail, + }) } if len(values) != 1 { - perr("expected %s for key %q, but key passed %v times", tipe, - kpath(key, keytail), len(values)) + panic(SingletonError{ + Key: kpath(key, keytail), + Type: tipe, + Values: values, + }) } } func keyed(tipe reflect.Type, key, keytail string) (string, string) { + if keytail[0] != '[' { + panic(SyntaxError{ + Key: kpath(key, keytail), + Subtype: MissingOpeningBrace, + ErrorPart: keytail, + }) + } + idx := strings.IndexRune(keytail, ']') - if keytail[0] != '[' || idx == -1 { - perr("expected a square bracket delimited index for %q "+ - "(of type %v)", kpath(key, keytail), tipe) + if idx == -1 { + panic(SyntaxError{ + Key: kpath(key, keytail), + Subtype: MissingClosingBrace, + ErrorPart: keytail[1:], + }) } + return keytail[1:idx], keytail[idx+1:] } func parseTextUnmarshaler(key, keytail string, values []string, target reflect.Value) { - primitive("encoding.TextUnmarshaler", key, keytail, values) + primitive(key, keytail, target.Type(), values) tu := target.Addr().Interface().(encoding.TextUnmarshaler) err := tu.UnmarshalText([]byte(values[0])) if err != nil { - perr("error while calling UnmarshalText on %v for key %q: %v", - target.Type(), kpath(key, keytail), err) + panic(TypeError{ + Key: kpath(key, keytail), + Type: target.Type(), + Err: err, + }) } } func parseBool(key, keytail string, values []string, target reflect.Value) { - primitive("bool", key, keytail, values) + primitive(key, keytail, target.Type(), values) switch values[0] { case "true", "1", "on": @@ -99,61 +120,77 @@ func parseBool(key, keytail string, values []string, target reflect.Value) { case "false", "0", "": target.SetBool(false) default: - perr("could not parse key %q as bool", kpath(key, keytail)) + panic(TypeError{ + Key: kpath(key, keytail), + Type: target.Type(), + }) } } func parseInt(key, keytail string, values []string, target reflect.Value) { - primitive("int", key, keytail, values) - t := target.Type() + primitive(key, keytail, t, values) + i, err := strconv.ParseInt(values[0], 10, t.Bits()) if err != nil { - perr("error parsing key %q as int: %v", kpath(key, keytail), - err) + panic(TypeError{ + Key: kpath(key, keytail), + Type: t, + Err: err, + }) } target.SetInt(i) } func parseUint(key, keytail string, values []string, target reflect.Value) { - primitive("uint", key, keytail, values) - t := target.Type() + primitive(key, keytail, t, values) + i, err := strconv.ParseUint(values[0], 10, t.Bits()) if err != nil { - perr("error parsing key %q as uint: %v", kpath(key, keytail), - err) + panic(TypeError{ + Key: kpath(key, keytail), + Type: t, + Err: err, + }) } target.SetUint(i) } func parseFloat(key, keytail string, values []string, target reflect.Value) { - primitive("float", key, keytail, values) - t := target.Type() + primitive(key, keytail, t, values) + f, err := strconv.ParseFloat(values[0], t.Bits()) if err != nil { - perr("error parsing key %q as float: %v", kpath(key, keytail), - err) + panic(TypeError{ + Key: kpath(key, keytail), + Type: t, + Err: err, + }) } target.SetFloat(f) } func parseString(key, keytail string, values []string, target reflect.Value) { - primitive("string", key, keytail, values) + primitive(key, keytail, target.Type(), values) target.SetString(values[0]) } func parseSlice(key, keytail string, values []string, target reflect.Value) { + t := target.Type() + // BUG(carl): We currently do not handle slices of nested types. If // support is needed, the implementation probably could be fleshed out. if keytail != "[]" { - perr("unexpected array nesting for key %q: %q", - kpath(key, keytail), keytail) + panic(NestingError{ + Key: kpath(key, keytail), + Type: t, + Nesting: keytail, + }) } - t := target.Type() slice := reflect.MakeSlice(t, len(values), len(values)) kp := kpath(key, keytail) diff --git a/param/struct.go b/param/struct.go index 314d5b0..8af3c08 100644 --- a/param/struct.go +++ b/param/struct.go @@ -108,8 +108,12 @@ func extractHandler(s reflect.Type, sf reflect.StructField) func(string, string, func parseStructField(cache structCache, key, sk, keytail string, values []string, target reflect.Value) { l, ok := cache[sk] if !ok { - perr("unknown key %q for struct at key %q", sk, - kpath(key, keytail)) + panic(KeyError{ + FullKey: key, + Key: kpath(key, keytail), + Type: target.Type(), + Field: sk, + }) } f := target.Field(l.offset)