From c73ef8800681e147e64139d190f75760cc8a99ff Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Tue, 28 Mar 2017 11:22:57 -0700 Subject: [PATCH 1/2] Don't panic when invalid paths are configured If the config struct includes invalid output paths, make sure that we don't try to execute a nil function pointer. Fixes #390. --- config_test.go | 22 ++++++++++++++++++++++ writer.go | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/config_test.go b/config_test.go index 65bcfb112..1c85dc775 100644 --- a/config_test.go +++ b/config_test.go @@ -84,3 +84,25 @@ func TestConfig(t *testing.T) { }) } } + +func TestConfigWithInvalidPaths(t *testing.T) { + tests := []struct { + desc string + output string + errOutput string + }{ + {"output directory doesn't exist", "/tmp/not-there/foo.log", "stderr"}, + {"error output directory doesn't exist", "stdout", "/tmp/not-there/foo-errors.log"}, + {"both output directories don't exist", "/tmp/not-there/foo.log", "/tmp/not-there/foo-errors.log"}, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + cfg := NewProductionConfig() + cfg.OutputPaths = []string{tt.output} + cfg.ErrorOutputPaths = []string{tt.errOutput} + _, err := cfg.Build() + assert.Error(t, err, "Expected an error opening a non-existent directory.") + }) + } +} diff --git a/writer.go b/writer.go index 238ca6f36..821fc9e64 100644 --- a/writer.go +++ b/writer.go @@ -38,7 +38,7 @@ import ( func Open(paths ...string) (zapcore.WriteSyncer, func(), error) { writers, close, err := open(paths) if err != nil { - return nil, nil, err + return nil, close, err } writer := CombineWriteSyncers(writers...) From a553907f8de2b87f99c23c0de8f30e32c3223c9e Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Tue, 28 Mar 2017 11:25:30 -0700 Subject: [PATCH 2/2] Small grammar fix --- config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config_test.go b/config_test.go index 1c85dc775..65cabdfd7 100644 --- a/config_test.go +++ b/config_test.go @@ -93,7 +93,7 @@ func TestConfigWithInvalidPaths(t *testing.T) { }{ {"output directory doesn't exist", "/tmp/not-there/foo.log", "stderr"}, {"error output directory doesn't exist", "stdout", "/tmp/not-there/foo-errors.log"}, - {"both output directories don't exist", "/tmp/not-there/foo.log", "/tmp/not-there/foo-errors.log"}, + {"neither output directory exists", "/tmp/not-there/foo.log", "/tmp/not-there/foo-errors.log"}, } for _, tt := range tests {