Skip to content

Commit

Permalink
solidate error handling and return checking
Browse files Browse the repository at this point in the history
  • Loading branch information
sio4 committed Apr 23, 2022
1 parent 6d217bf commit 520a801
Show file tree
Hide file tree
Showing 15 changed files with 75 additions and 32 deletions.
8 changes: 6 additions & 2 deletions checks/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ func (p *Checker) Run(c common.Context, opts common.PluginOptions, ch chan inter
return p.runFunc(c, opts, ch)
}

func StartAll(c common.Context, opts *common.Options, ch chan interface{}) {
func StartAll(c common.Context, opts *common.Options, ch chan interface{}) int {
logger := c.Logger().WithField("module", "checker")
n := 0

for _, x := range common.Plugins(reflect.TypeOf(&Checker{})) {
x := x.(common.Plugin)
x, _ := x.(common.Plugin)
if len(opts.Checkers) > 0 && !common.Contains(opts.Checkers, x.Name()) {
logger.Debugf("%v is not on the checker list. skipping...", x.Name())
continue
Expand All @@ -44,6 +45,9 @@ func StartAll(c common.Context, opts *common.Options, ch chan interface{}) {
if err := x.Run(c, copts, ch); err != nil {
logger.Errorf("%s checker was aborted: %v", x.Name(), err)
// TODO: should returns error?
} else {
n++
}
}
return n
}
7 changes: 5 additions & 2 deletions checks/checks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ func TestStartAll(t *testing.T) {
ctx, _ := common.NewDefaultContext(&opts)
defer ctx.Cancel()

StartAll(ctx, &opts, ctx.Channel())
n := StartAll(ctx, &opts, ctx.Channel())
r.Equal(1, n)

out := <-ctx.Channel()
r.NotNil(out)
Expand All @@ -41,6 +42,8 @@ func TestStartAll_Error(t *testing.T) {
ctx, _ := common.NewDefaultContext(&opts)
defer ctx.Cancel()

StartAll(ctx, &opts, ctx.Channel())
n := StartAll(ctx, &opts, ctx.Channel())
r.Equal(0, n)

r.Equal("--- &{{} [0 0 0]}", fmt.Sprintf("--- %v", ctx.WG()))
}
4 changes: 2 additions & 2 deletions checks/heartbeat.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package checks

import (
"errors"
"fmt"
"time"

"github.com/hyeoncheon/bogo/internal/common"
Expand All @@ -24,7 +24,7 @@ func heartbeatRunner(c common.Context, opts common.PluginOptions, out chan inter

interval, err := opts.GetIntegerOr("interval", heartbeatCheckerIntervalSec)
if err != nil {
return errors.New("invalid option value: interval")
return fmt.Errorf("%w: interval", common.ErrInvalidOptionValue)
}

c.WG().Add(1)
Expand Down
14 changes: 10 additions & 4 deletions checks/ping.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package checks

import (
"errors"
"fmt"
"time"

Expand All @@ -17,6 +18,11 @@ const (
pingCount = 10
)

var (
errNoTargetsSpecified = errors.New("no targets specified")
errTargetStringShouldNotBeEmpty = errors.New("target string should not be empty")
)

func (*Checker) RegisterPing() *Checker {
return &Checker{
name: pingChecker,
Expand All @@ -35,12 +41,12 @@ func pingRunner(c common.Context, opts common.PluginOptions, out chan interface{

checkInterval, err := opts.GetIntegerOr("check_interval", pingCheckerIntervalSec)
if err != nil {
return fmt.Errorf("invalid option value: check_interval")
return fmt.Errorf("%w: check_interval", common.ErrInvalidOptionValue)
}

pingInterval, err := opts.GetIntegerOr("ping_interval", pingIntervalMilliSec)
if err != nil {
return fmt.Errorf("invalid option value: check_interval")
return fmt.Errorf("%w: check_interval", common.ErrInvalidOptionValue)
}

// spawn ping workers for each target
Expand Down Expand Up @@ -82,12 +88,12 @@ func getTarget(c common.Context, opts *common.PluginOptions) ([]string, error) {
}

if len(targets) < 1 {
return targets, fmt.Errorf("no targets specified")
return targets, errNoTargetsSpecified
}

for _, t := range targets {
if len(t) < 1 {
return targets, fmt.Errorf("target string should not be empty: %v", targets)
return targets, fmt.Errorf("%w: %v", errTargetStringShouldNotBeEmpty, targets)
}
}

Expand Down
8 changes: 5 additions & 3 deletions cmd/bogo/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"context"
"errors"
"fmt"
"net/http"
"os"
Expand Down Expand Up @@ -81,8 +82,9 @@ func run(c common.Context, opts *common.Options) {
}
}()

checks.StartAll(c, opts, c.Channel())
exporters.StartAll(c, opts, c.Channel())
cn := checks.StartAll(c, opts, c.Channel())
en := exporters.StartAll(c, opts, c.Channel())
logger.Infof("%d checkers and %d exporters started", cn, en)

server, err := startWebRoutine(c, opts)
if err != nil {
Expand Down Expand Up @@ -118,7 +120,7 @@ func startWebRoutine(c common.Context, opts *common.Options) (meari.Server, erro
defer c.WG().Done()

err := server.Start()
if err == http.ErrServerClosed {
if errors.Is(err, http.ErrServerClosed) {
logger.Info("webserver closed")
} else {
logger.Error("unexpected error: ", err)
Expand Down
4 changes: 2 additions & 2 deletions cmd/bogo/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ func TestRun(t *testing.T) {
}()

time.Sleep(1 * time.Second)
syscall.Kill(syscall.Getpid(), syscall.SIGUSR1)
_ = syscall.Kill(syscall.Getpid(), syscall.SIGUSR1)

syscall.Kill(syscall.Getpid(), syscall.SIGINT)
_ = syscall.Kill(syscall.Getpid(), syscall.SIGINT)
wg.Wait()
r.True(indicator)
}
8 changes: 6 additions & 2 deletions exporters/exporters.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@ func (p *Exporter) Run(c common.Context, opts common.PluginOptions, ch chan inte
return p.runFunc(c, opts, ch)
}

func StartAll(c common.Context, opts *common.Options, ch chan interface{}) {
func StartAll(c common.Context, opts *common.Options, ch chan interface{}) int {
logger := c.Logger().WithField("module", "exporter")
n := 0

for _, x := range common.Plugins(reflect.TypeOf(&Exporter{})) {
x := x.(common.Plugin)
x, _ := x.(common.Plugin)
if len(opts.Exporters) > 0 && !common.Contains(opts.Exporters, x.Name()) {
logger.Debugf("%v is not on the exporter list. skipping...", x.Name())
continue
Expand All @@ -39,6 +40,9 @@ func StartAll(c common.Context, opts *common.Options, ch chan interface{}) {
if err := x.Run(c, eopts, ch); err != nil {
logger.Errorf("%s exporter was aborted: %v", x.Name(), err)
// TODO: should returns error?
} else {
n++
}
}
return n
}
6 changes: 5 additions & 1 deletion exporters/exporters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ import (
"testing"

"github.com/hyeoncheon/bogo/internal/common"

"github.com/stretchr/testify/require"
)

func TestStartAll(t *testing.T) {
r := require.New(t)
opts := common.DefaultOptions()
opts.Exporters = []string{"stdout"}
opts.ExporterOptions = map[string]common.PluginOptions{
Expand All @@ -16,7 +19,8 @@ func TestStartAll(t *testing.T) {
}
ctx, _ := common.NewDefaultContext(&opts)

StartAll(ctx, &opts, ctx.Channel())
n := StartAll(ctx, &opts, ctx.Channel())
r.Equal(1, n)

ctx.Channel() <- "message"

Expand Down
12 changes: 6 additions & 6 deletions exporters/stackdriver.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func stackdriverRunner(c common.Context, _ common.PluginOptions, in chan interfa
}

if err := registerViews(); err != nil {
return fmt.Errorf("could not register views: %v", err)
return fmt.Errorf("could not register views: %w", err)
}

exporter, err := createAndStartExporter()
Expand Down Expand Up @@ -77,7 +77,7 @@ func stackdriverRunner(c common.Context, _ common.PluginOptions, in chan interfa
if pm, ok := m.(bogo.PingMessage); ok {
logger.Debugf("ping: %v", pm)
if err := recordPingMessage(r, &pm); err != nil {
logger.Errorf("message %v: %v", pm, err)
logger.Errorf("message %v: %w", pm, err)
}
} else {
logger.Warnf("unknown: %v", m)
Expand All @@ -95,7 +95,7 @@ func getReporter(c common.Context) (*reporter, error) {
// currently, stackdriver exporter is only suppored on the GCE instance
meta := c.Meta()
if meta == nil || meta.WhereAmI() != "Google" {
return nil, fmt.Errorf("not on the Google Compute Engine")
return nil, common.ErrorNotOnGCE
}

return &reporter{
Expand Down Expand Up @@ -146,11 +146,11 @@ func createAndStartExporter() (*stackdriver.Exporter, error) {
},
})
if err != nil {
return nil, fmt.Errorf("could not create exporter: %v", err)
return nil, fmt.Errorf("could not create exporter: %w", err)
}

if err := exporter.StartMetricsExporter(); err != nil {
return nil, fmt.Errorf("could not start metric exporter: %v", err)
return nil, fmt.Errorf("could not start metric exporter: %w", err)
}
return exporter, nil
}
Expand All @@ -166,7 +166,7 @@ func recordPingMessage(r *reporter, m *bogo.PingMessage) error {
avgRttMs.M(float64(m.AvgRtt.Milliseconds())),
lossRate.M(m.Loss),
); err != nil {
return fmt.Errorf("could not send ping stat: %v", err)
return fmt.Errorf("could not send ping stat: %w", err)
}
return nil
}
3 changes: 3 additions & 0 deletions internal/common/gcp.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package common

import (
"errors"
"net/http"
"strings"
"sync"
Expand All @@ -12,6 +13,8 @@ import (

const GOOGLE = "Google"

var ErrorNotOnGCE = errors.New("not on the Google Compute Engine")

// asset GCEClient for MetaClient iplemetations
var _ MetaClient = &GCEClient{}

Expand Down
2 changes: 1 addition & 1 deletion internal/common/gcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func TestGCEClient_RoundTrip(t *testing.T) {

server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
ua := req.Header.Get("User-Agent")
rw.Write([]byte(ua))
_, _ = rw.Write([]byte(ua))
}))
defer server.Close()

Expand Down
8 changes: 7 additions & 1 deletion internal/common/plugins.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
package common

import (
"errors"
"fmt"
"reflect"
"strconv"
"strings"
)

var (
ErrInvalidePluginOption = errors.New("invalid plugin option")
ErrInvalidOptionValue = errors.New("invalid option value")
)

type Plugin interface {
Name() string
Run(Context, PluginOptions, chan interface{}) error
Expand Down Expand Up @@ -59,7 +65,7 @@ func BuildPluginOptions(s string) (map[string]PluginOptions, error) {
options[t[0]] = PluginOptions{t[1]: StringValues(t[2])}
}
} else {
return nil, fmt.Errorf("invalid plugin option '%v'", e)
return nil, fmt.Errorf("%w: %v", ErrInvalidePluginOption, e)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/common/plugins_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package common

import (
"errors"
"fmt"
"reflect"
"testing"

Expand Down Expand Up @@ -63,12 +63,12 @@ func TestBuildPluginOptions(t *testing.T) {
{
in: "heartbeat:interval",
out: nil,
e: errors.New("invalid plugin option 'heartbeat:interval'"),
e: fmt.Errorf("%w: heartbeat:interval", ErrInvalidePluginOption),
},
{
in: "heartbeat",
out: nil,
e: errors.New("invalid plugin option 'heartbeat'"),
e: fmt.Errorf("%w: heartbeat", ErrInvalidePluginOption),
},
}

Expand Down
7 changes: 6 additions & 1 deletion makefile
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@

LINTERS = gocognit,gocyclo,misspell
MORE_LINTERS = errorlint,goerr113,forcetypeassert
DISABLED_LINTERS = godot,godox,wsl,varnamelen,nlreturn,testpackage,paralleltest,wrapcheck,exhaustivestruct,gci,gochecknoglobals

default: test lint

lint:
golangci-lint run -E gocognit,gocyclo,misspell --tests=false
golangci-lint run -E $(LINTERS)
golangci-lint run -E $(LINTERS),$(MORE_LINTERS) --tests=false

test:
go test -race -coverprofile=coverage.txt -covermode=atomic ./...
10 changes: 8 additions & 2 deletions meari/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package meari

import (
"context"
"errors"
"fmt"
"net/http"

Expand All @@ -11,6 +12,11 @@ import (
"github.com/hyeoncheon/bogo/internal/common"
)

var (
errorCouldNotInitiate = errors.New("could not initiate the web server")
errorUnsupportedMethod = errors.New("unsupported method")
)

type Options struct {
Logger common.Logger
Address string
Expand All @@ -32,7 +38,7 @@ func NewServer(c common.Context, opts *common.Options) (Server, error) {
}
server := NewDefaultServer(serverOpts)
if server == nil {
return nil, fmt.Errorf("could not initiate the web server: %v", serverOpts)
return nil, fmt.Errorf("%w: %v", errorCouldNotInitiate, serverOpts)
}

for p, handler := range handlers.AllHandlers() {
Expand All @@ -41,7 +47,7 @@ func NewServer(c common.Context, opts *common.Options) (Server, error) {
logger.Debugf("register handler for 'GET %v'...", p)
server.GET(p, handler.Handler)
default:
return nil, fmt.Errorf("unsupported method for %v", handler)
return nil, fmt.Errorf("%w: %v", errorUnsupportedMethod, handler)
}
}

Expand Down

0 comments on commit 520a801

Please sign in to comment.