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.

Sign up for GitHub

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

Jump to bottom

feat: support short name for HTTP Errors #8

Merged
merged 1 commit into from
Dec 26, 2018
Merged

feat: support short name for HTTP Errors #8

merged 1 commit into from
Dec 26, 2018

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Dec 23, 2018

e.g.:

  • NotFoundError alias to E404
  • ImATeapotError alias to E418

@fengmk2
feat: support short name for HTTP Errors
7362dd1 
e.g.:

- NotFoundError alias to E404
- ImATeapotError alias to E418
@fengmk2 fengmk2 added the enhancement New feature or request label Dec 23, 2018
@fengmk2
Copy link
Member Author

fengmk2 commented Dec 23, 2018

cc @dead-horse eggjs/egg#1086 (comment)

@codecov
Copy link

codecov bot commented Dec 23, 2018 •
edited
Loading

Codecov Report

Merging #8 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #8   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          50     50           
  Lines         342    342           
  Branches       48     48           
=====================================
  Hits          342    342
Impacted Files Coverage Δ
lib/index.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1697867...7362dd1. Read the comment docs.

fengmk2
fengmk2 commented Dec 23, 2018
View reviewed changes
test/http/416.test.ts

it('should alias to short name E416', () => {
const err = new E416();
assert(err.code === 'RANGE_NOT_SATISFIABLE');
Copy link
Member Author

Choose a reason for hiding this comment

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

code 是否统一加 ERR_EGG_HTTP_ 前缀比较好呢？

Copy link
Member

Choose a reason for hiding this comment

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

可以加个前缀，不过要发个大版本

Copy link
Member Author

Choose a reason for hiding this comment

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

那还是这样吧，目前 code 也没有系统性收集。

Copy link
Member

Choose a reason for hiding this comment

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

问题应该也不大，如果我们后面要供统一的处理方法。只需要保证在 egg-errors 体系内的错误码不重复定义就好了。

@fengmk2
Copy link
Member Author

fengmk2 commented Dec 26, 2018

@popomore 来合并？或者给我加一下权限

@popomore popomore merged commit 10d565a into master Dec 26, 2018
@popomore popomore deleted the support-short-name branch December 26, 2018 13:29
@popomore
Copy link
Member

  • egg-errors@2.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dead-horse dead-horse dead-horse approved these changes

@popomore popomore popomore approved these changes

Assignees

@dead-horse dead-horse

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@fengmk2 @popomore @dead-horse