Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: more precise and detailed handling of errors #111

Merged
merged 3 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions internal/paper/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import (
"context"
"errors"
"fmt"

"github.com/west2-online/fzuhelper-server/internal/paper/service"
"github.com/west2-online/fzuhelper-server/kitex_gen/model"
Expand Down Expand Up @@ -47,12 +48,12 @@

success, fileDir, err = service.NewPaperService(ctx, s.ClientSet).GetDir(req)
if !success {
logger.Infof("Paper.ListDirFiles: get dir info failed from upyun")
logger.Errorf("Paper.ListDirFiles: get dir info failed from upyun")

Check warning on line 51 in internal/paper/handler.go

View check run for this annotation

Codecov / codecov/patch

internal/paper/handler.go#L51

Added line #L51 was not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

因为原来的代码并没有对error的内容进行输出,所以这里也没有更改逻辑,只更改了日志等级

resp.Base = base.BuildBaseResp(errors.New("failed to get info from upyun"))
return resp, nil
}
if err != nil {
logger.Infof("Paper.ListDirFiles: get dir info failed: %v", err)
base.LogError(fmt.Errorf("Paper.ListDirFiles: get dir info failed: %w", err))

Check warning on line 56 in internal/paper/handler.go

View check run for this annotation

Codecov / codecov/patch

internal/paper/handler.go#L56

Added line #L56 was not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

将正常的err中的信息的日志集成进了 base.LogerError中,这里用fmt.Errorf() 对err再包装是因为原来就输出了这些信息,而我不打算更改原有的输出内容

resp.Base = base.BuildBaseResp(errors.New("failed to get info from upyun"))
return resp, nil
}
Expand All @@ -68,8 +69,7 @@

url, err := service.NewPaperService(ctx, s.ClientSet).GetDownloadUrl(req)
if err != nil {
logger.Infof("Paper.GetDownloadUrl: get download url failed: %v", err)
resp.Base = base.BuildBaseResp(err)
resp.Base = base.BuildRespAndLog(fmt.Errorf("Paper.GetDownloadUrl: get download url failed: %w", err))

Check warning on line 72 in internal/paper/handler.go

View check run for this annotation

Codecov / codecov/patch

internal/paper/handler.go#L72

Added line #L72 was not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里是另一种日志输出err中信息的方法,他将日志与resp的build集成在了一起,减轻开发者心智负担

return resp, nil
}

Expand Down
5 changes: 4 additions & 1 deletion internal/paper/service/get_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
return false, nil, fmt.Errorf("service.GetDir: get dir info failed: %w", err)
}

err = s.cache.Paper.SetFileDirCache(s.ctx, key, *fileDir)
if err = s.cache.Paper.SetFileDirCache(s.ctx, key, *fileDir); err != nil {
return true, fileDir, fmt.Errorf("service.GetDir: set file dir cache failed: %w", err)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个地方并不产生错误,他只是错误的传递者之一,于是使用fmt.Errorf()对其进行了错误链的封装

}

Check warning on line 54 in internal/paper/service/get_dir.go

View check run for this annotation

Codecov / codecov/patch

internal/paper/service/get_dir.go#L52-L54

Added lines #L52 - L54 were not covered by tests

return true, fileDir, err
}
24 changes: 22 additions & 2 deletions pkg/base/pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,28 @@

e := errno.ConvertErr(err)
if e.StackTrace() != nil {
logger.Errorf("%s\n%+v", err.Error(), e.StackTrace())
logger.LErrorf("%v\nStacktrace:%+v\n", err, e.StackTrace())

Check warning on line 50 in pkg/base/pack.go

View check run for this annotation

Codecov / codecov/patch

pkg/base/pack.go#L50

Added line #L50 was not covered by tests
return
}
logger.Errorf("%s\n", err.Error())
logger.LErrorf("%v\n", err)

Check warning on line 53 in pkg/base/pack.go

View check run for this annotation

Codecov / codecov/patch

pkg/base/pack.go#L53

Added line #L53 was not covered by tests
}

func BuildRespAndLog(err error) *model.BaseResp {
if err == nil {
return &model.BaseResp{
Code: errno.SuccessCode,
Msg: errno.Success.ErrorMsg,
}
}

Check warning on line 62 in pkg/base/pack.go

View check run for this annotation

Codecov / codecov/patch

pkg/base/pack.go#L56-L62

Added lines #L56 - L62 were not covered by tests

Errno := errno.ConvertErr(err)
if Errno.StackTrace() != nil {
logger.LErrorf("%v\nStacktrace:%+v\n", err, Errno.StackTrace())
} else {
logger.LErrorf("%v\n", err)
}
return &model.BaseResp{
Code: Errno.ErrorCode,
Msg: Errno.ErrorMsg,
}

Check warning on line 73 in pkg/base/pack.go

View check run for this annotation

Codecov / codecov/patch

pkg/base/pack.go#L64-L73

Added lines #L64 - L73 were not covered by tests
}
6 changes: 3 additions & 3 deletions pkg/cache/paper/get_file_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,23 @@

import (
"context"
"fmt"

"github.com/bytedance/sonic"

"github.com/west2-online/fzuhelper-server/kitex_gen/model"
"github.com/west2-online/fzuhelper-server/pkg/errno"
)

func (c *CachePaper) GetFileDirCache(ctx context.Context, key string) (bool, *model.UpYunFileDir, error) {
ret := &model.UpYunFileDir{}

data, err := c.client.Get(ctx, key).Bytes()
if err != nil {
return false, ret, fmt.Errorf("dal.GetFileDirCache: get dir info failed: %w", err)
return false, ret, errno.Errorf(errno.InternalDatabaseErrorCode, "dal.GetFileDirCache: get dir info failed: %v", err)

Check warning on line 33 in pkg/cache/paper/get_file_dir.go

View check run for this annotation

Codecov / codecov/patch

pkg/cache/paper/get_file_dir.go#L33

Added line #L33 was not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的错误是origin error,也就是产生错误的地方,所以我们使用errno中的函数来创建error,这样的话这个error会包含有当前的栈信息。

}
err = sonic.Unmarshal(data, &ret)
if err != nil {
return false, ret, fmt.Errorf("dal.GetFileDirCache: Unmarshal dir info failed: %w", err)
return false, ret, errno.Errorf(errno.InternalJSONErrorCode, "dal.GetFileDirCache: Unmarshal dir info failed: %v", err)

Check warning on line 37 in pkg/cache/paper/get_file_dir.go

View check run for this annotation

Codecov / codecov/patch

pkg/cache/paper/get_file_dir.go#L37

Added line #L37 was not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上,产生错误的地方需要使用errno来包装一个栈信息

}
return true, ret, nil
}
9 changes: 6 additions & 3 deletions pkg/cache/paper/set_file_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,23 @@

import (
"context"
"fmt"

"github.com/bytedance/sonic"

"github.com/west2-online/fzuhelper-server/kitex_gen/model"
"github.com/west2-online/fzuhelper-server/pkg/constants"
"github.com/west2-online/fzuhelper-server/pkg/errno"
)

const TwoDay = 2 * constants.OneDay

func (c *CachePaper) SetFileDirCache(ctx context.Context, key string, dir model.UpYunFileDir) error {
data, err := sonic.Marshal(dir)
if err != nil {
return fmt.Errorf("dal.SetFileDirCache: Unmarshal dir info failed: %w", err)
return errno.Errorf(errno.InternalJSONErrorCode, "dal.SetFileDirCache: Unmarshal dir info failed: %v", err)

Check warning on line 34 in pkg/cache/paper/set_file_dir.go

View check run for this annotation

Codecov / codecov/patch

pkg/cache/paper/set_file_dir.go#L34

Added line #L34 was not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

产生error的地方

}
return c.client.Set(ctx, key, data, TwoDay).Err()
if err = c.client.Set(ctx, key, data, TwoDay).Err(); err != nil {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

产生

return errno.Errorf(errno.InternalDatabaseErrorCode, "%v", err)
}
return nil

Check warning on line 39 in pkg/cache/paper/set_file_dir.go

View check run for this annotation

Codecov / codecov/patch

pkg/cache/paper/set_file_dir.go#L36-L39

Added lines #L36 - L39 were not covered by tests
}
5 changes: 5 additions & 0 deletions pkg/logger/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,8 @@
func Fatalf(template string, args ...interface{}) {
klog.Fatalf(template, args...)
}

// LErrorf Equals Errorf less one stack
func LErrorf(template string, args ...interface{}) {
loggerObj.Errorf(template, args...)

Check warning on line 65 in pkg/logger/output.go

View check run for this annotation

Codecov / codecov/patch

pkg/logger/output.go#L64-L65

Added lines #L64 - L65 were not covered by tests
}
Loading