Merge pull request #59 from dyweb/errors/init

[errors] Replace pkg/errors Fix #54 

- multi error and wrap error
- multi error `Append(err error)` returns true if it got a non nil error uber-go/multierr#21
- `Wrap` would reuse stack if the underlying error has one pkg/errors#122, and if `Frames()` is not called, `runtime.Frames` would not be expanded to `[]runtime.Frame`

.ayi.yml

@@ -1,10 +1,10 @@
 dep-install:
     - dep ensure
 test:
-    - go test -v -cover ./cast/... ./config/... ./generator/... ./log/... ./requests/... ./structure/... ./util/...
+    - go test -v -cover ./cast/... ./config/... ./errors/... ./generator/... ./log/... ./noodle/... ./requests/... ./structure/... ./util/...
 scripts:
   fmt:
-    - gofmt -d -l -w ./cast ./config ./log ./requests ./structure ./util
+    - gofmt -d -l -w ./cast ./config ./errors ./log ./requests ./structure ./util
     - git status
   check:
     # TODO: didn't see any error so far ...

.travis.yml

@@ -13,5 +13,7 @@ install:
 
 script:
   - make install
-  - go test -v -cover ./cast/... ./config/... ./generator/... ./log/... ./noodle/... ./requests/... ./runner/... ./structure/... ./util/...
+  - make test
+  - make test-race
+  - make vet
   # TODO: gommon doesn't have coverage ... amazing ..

Gopkg.toml

@@ -27,13 +27,11 @@
 
 ignored = [
     "github.com/dyweb/gommon/legacy",
+    "github.com/dyweb/gommon/legacy/*",
     "github.com/dyweb/gommon/playground",
+    "github.com/dyweb/gommon/playground/*",
 ]
 
-[[constraint]]
-  name = "github.com/pkg/errors"
-  version = "0.8.0"
-
 [prune]
   go-tests = true
   unused-packages = true

Makefile

@@ -1,18 +1,34 @@
+PKGS=./cast/... ./config/... ./errors/... ./generator/... ./log/... ./noodle/... ./requests/... ./structure/... ./util/...
+PKGST=./cast ./config ./errors ./generator ./log ./noodle ./requests ./structure ./util
+
 .PHONY: install
 install:
 	go install ./cmd/gommon
 
 .PHONY: test
 test:
-	go test -v -cover ./cast/... ./config/... ./generator/... ./log/... ./noodle/... ./requests/... ./structure/... ./util/...
+	go test -v -cover $(PKGS)
+
+.PHONY: test-race
+test-race:
+	go test -race $(PKGS)
 
 .PHONY: test-log
 test-log:
 	go test -v -cover ./log/...
 
 .PHONY: fmt
 fmt:
-	gofmt -d -l -w ./cast ./config ./generator ./log ./noodle ./requests ./structure ./util
+	gofmt -d -l -w $(PKGST)
+
+.PHONY: vet
+vet:
+	go vet $(PKGST)
+
+.PHONy: doc
+doc:
+	xdg-open http://localhost:6060/pkg/github.com/dyweb/gommon &
+	godoc -http=":6060"
 
 .PHONY: docker-test
 docker-test:

README.md

@@ -10,13 +10,14 @@ Gommon is a collection of common util libraries written in Go.
 
 It has the following components:
 
-- [Config](config) A YAML config reader with template support
-- [Log](log) A Javaish logger for Go, application can control library and set level for different pkg via config or flag
-- [Generator](generator) Render go template, generate methods for logger interface based on `gommon.yml`
-- [Noodle](noodle) Embed static assets for web application with `.noodleignore` support
-- [Requests](requests) A pythonic wrapper for `net/http`, HTTP for Gopher.
-- [Cast](cast) Convert Golang types
-- [Data structure](structure) Bring Set etc. to Golang.
+- [config](config) A YAML config reader with template support
+- [errors](errors) Wrap error and multi error
+- [generator](generator) Render go template, generate methods for logger interface based on `gommon.yml`
+- [log](log) A Javaish logger for Go, application can control library and set level for different pkg via config or flag
+- [noodle](noodle) Embed static assets for web application with `.noodleignore` support
+- [requests](requests) A pythonic wrapper for `net/http`, HTTP for Gopher.
+- [cast](cast) Convert Golang types
+- [structure](structure) Bring data structure like Set etc. to Golang.
 
 Legacy
 
@@ -26,16 +27,18 @@ Legacy
 
 ## Dependencies
 
-Currently we only have two non standard library dependencies, see [Gopkg.lock](Gopkg.lock), 
-we might replace them with our own implementation in the future.
+Currently we only have one non standard library dependencies, see [Gopkg.lock](Gopkg.lock)
 
-- [pkg/errors](https://github.com/pkg/errors) for including context in error
-  - however it's still not very machine friendly, we are likely to opt it out in future version
 - [go-yaml/yaml](https://github.com/go-yaml/yaml) for read config written in YAML
   - we don't need most feature of YAML, and want to have access to the parser directly to report which line has incorrect semantic (after checking it in application).
     - might write one in [ANTLR](https://github.com/antlr/antlr4)
   - we also have a DSL work in progress [RCL: Reika Configuration Language](https://github.com/at15/reika/issues/49), which is like [HCL](https://github.com/hashicorp/hcl2)
 
+Removed 
+
+- [pkg/errors](https://github.com/pkg/errors) for including context in error
+  - removed in [#59](https://github.com/dyweb/gommon/pull/59)
+
 <!-- no, we are using the standard flag package ... -->
 <!-- For command line util, we are using [spf13/cobra](https://github.com/spf13/cobra), it is more flexible than [ufrave/cli](https://github.com/urfave/cli) -->
 
@@ -66,23 +69,46 @@ Currently, gommon is in a very violate state, please open issues after it become
 Gommon is inspired by the following awesome libraries, most gommon packages have much less features and a few improvements 
 compared to packages it modeled after.
 
+log
+
 - [sirupsen/logrus](https://github.com/sirupsen/logrus) for structured logging 
   - log v1 is entirely modeled after logrus, entry contains log information with methods like `Info`, `Infof`
 - [apex/log](https://github.com/apex/log) for log handlers
   - log v2's handler is inspired by apex/log, but we didn't use entry and chose to pass multiple parameters to explicitly state what a handler should handle
 - [uber-go/zap](https://github.com/uber-go/zap) for serialize log fields without using `fmt.Sprintf` and use `strconv` directly
   - we didn't go that extreme as Zap or ZeroLog for zero allocation, performance is not our goal for now
+
+config
+
 - [spf13/cast](https://github.com/spf13/cast) for cast, it is used by Viper
 - [spf13/viper](https://github.com/spf13/viper/) for config
   - looking up config via string key makes type system useless, so we always marshal entire config file to a single struct
     - it also makes refactor easier
+
+requests
+
 - [Requests](http://docs.python-requests.org/en/master/) for requests
+- [hashicorp/go-cleanhttp](https://github.com/hashicorp/go-cleanhttp) for using non default http transport and client
+
+generator
+
 - [benbjohnson/tmpl](https://github.com/benbjohnson/tmpl) for go template generator
   - first saw it in [influxdata/influxdb](https://github.com/influxdata/influxdb/blob/master/tsdb/engine/tsm1/encoding.gen.go.tmpl)
   - we put template data in `gommon.yml`, so we don't need to pass data as json via cli
-- [GeertJohan/go.rice](https://github.com/GeertJohan/go.rice) for ~~rice~~ noodle
+
+noodle
+
+- [GeertJohan/go.rice](https://github.com/GeertJohan/go.rice)
   - we implemented `.gitignore` like [feature](https://github.com/at15/go.rice/issues/1) but the upstream didn't respond for the [feature request #83](https://github.com/GeertJohan/go.rice/issues/83)
 
+errors
+
+- [pkg/errors](https://github.com/pkg/errors) it can not introduce breaking change, but `WithMessage` and `WithStack` is annoying
+  - see [#54](https://github.com/dyweb/gommon/issues/54) and [errors/doc](errors/doc) about other error packages
+  - https://github.com/pkg/errors/pull/122 for check existing stack before attach new one
+- [uber-go/multierr#21]( https://github.com/uber-go/multierr/issues/21) for return bool after append
+- [hashicorp/go-multierror](https://github.com/hashicorp/go-multierror) for `ErrorOrNil`
+
 ## About
 
 It was part of [Ayi](https://github.com/dyweb/Ayi) and split out for wider use.

cast/cast.go

@@ -2,7 +2,8 @@ package cast
 
 import (
 	"encoding/json"
-	"github.com/pkg/errors"
+
+	"github.com/dyweb/gommon/errors"
 	"gopkg.in/yaml.v2"
 )
 

cast/pkg.go

@@ -1 +1,4 @@
+// Package cast convert types safely and drop incompatible types during conversion
+//
+// it is inspired by https://github.com/spf13/cast
 package cast

cmd/gommon/main.go

@@ -1,3 +1,4 @@
+// gommon is the commandline util for generator
 package main
 
 import (

config/pkg.go

@@ -1,3 +1,4 @@
+// Package config supports go text/template, environment and self defined variables
 package config
 
 import (

config/yaml.go

@@ -2,19 +2,19 @@ package config
 
 import (
 	"bytes"
-	"fmt"
 	"io/ioutil"
 	"os"
 	"reflect"
 	"strings"
 	"sync"
 	"text/template"
 
+	"gopkg.in/yaml.v2"
+
 	"github.com/dyweb/gommon/cast"
+	"github.com/dyweb/gommon/errors"
 	dlog "github.com/dyweb/gommon/log"
 	"github.com/dyweb/gommon/util"
-	"github.com/pkg/errors"
-	"gopkg.in/yaml.v2"
 )
 
 // YAMLConfig is a thread safe struct for parse YAML file and get value
@@ -35,10 +35,10 @@ func LoadYAMLAsStruct(file string, structuredConfig interface{}) error {
 	}
 	c := NewYAMLConfig()
 	if err := c.ParseSingleDocument(b); err != nil {
-		return errors.WithMessage(err, "can't parse as single document")
+		return errors.Wrap(err, "can't parse as single document")
 	}
 	if err := c.Unmarshal(structuredConfig, true); err != nil {
-		return errors.WithMessage(err, fmt.Sprintf("can't turn file %s into structured config", file))
+		return errors.Wrapf(err, "can't turn file %s into structured config", file)
 	}
 	return nil
 }

errors/README.md

@@ -0,0 +1,5 @@
+# Errors
+
+A error package supports wrapping and multi error
+
+- init https://github.com/dyweb/gommon/issues/54

errors/doc/README.md

@@ -0,0 +1,32 @@
+# Doc
+
+## Survey
+
+wrap
+
+- [pkg/errors](pkg-errors.md)
+- [juju/errgo](juju-errgo.md)
+  - support `Mask`, which is not seen in other packages
+- [juju/errors](juju-errors.md)
+  - same as errgo, and not depending on it, the description is [outdated](https://github.com/juju/errors/pull/10)
+- [hashicorp/errwrap](hashicorp-errwrap.md)
+  - contains string or type
+  - get error by string or type
+
+multi error
+
+- [hashicorp/go-multierror](hashicorp-go-multierror.md)
+  - flatten multierror when append
+  - ErrorOrNil
+    - https://golang.org/doc/faq#nil_error Why is my nil error value not equal to nil?
+- [uber/multierr](uber-multierr.md)
+  - combine more than one error
+  - [x] has a `atomic.Book` for copyNeeded, seems only used for fast path
+
+TODO: k8s
+
+- [ ] https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/errors/errors.go
+- [ ] https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go
+
+
+- implement Formatter interface `Format(f State, c rune)` https://golang.org/pkg/fmt/#Formatter

errors/doc/hashicorp-errwrap.md

@@ -0,0 +1,45 @@
+# hashicorp/errwrap
+
+- repo: https://github.com/hashicorp/errwrap
+- doc: https://godoc.org/github.com/hashicorp/errwrap
+- last update: 2014/10/07
+
+Usage
+
+- wrap
+- check contains
+- get error(s) based on string/type
+- walk
+
+````go
+func Contains(err error, msg string) bool
+func ContainsType(err error, v interface{}) bool
+func GetAll(err error, msg string) []error
+func GetAllType(err error, v interface{}) []error
+func Walk(err error, cb WalkFunc)
+func Wrap(outer, inner error) error
+func Wrapf(format string, err error) error
+type WalkFunc func(error)
+type Wrapper interface {
+    WrappedErrors() []error
+}
+````
+
+- I was thinking it is using `strings.Contains` but it is actually comparing message directly ...
+
+````go
+// GetAll gets all the errors that might be wrapped in err with the
+// given message. The order of the errors is such that the outermost
+// matching error (the most recent wrap) is index zero, and so on.
+func GetAll(err error, msg string) []error {
+	var result []error
+
+	Walk(err, func(err error) {
+		if err.Error() == msg {
+			result = append(result, err)
+		}
+	})
+
+	return result
+}
+````