Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

error_message: more friendly error message #711 #799

Merged
merged 3 commits into from
Jul 15, 2020

Conversation

GMHDBJD
Copy link
Collaborator

@GMHDBJD GMHDBJD commented Jul 10, 2020

manually cherrypick #771

What problem does this PR solve?

  • use TError to replace ProcessError
     "errors": [
                              {
                                  "msg": "",
                                  "error": {
                                      "ErrCode": 10006,
                                      "ErrClass": 1,
                                      "ErrScope": 2,
                                      "ErrLevel": 3,
                                      "Message": "this is the error message",
                                      "RawCause": "this is the raw cause",
                                      "Workaround": "this is the workaround"
                                  }
                              }
             ]
    
    will become
    "errors": [
                              {
                                  "ErrCode": 10006,
                                  "ErrClass": "database",
                                  "ErrScope": "downstream",
                                  "ErrLevel": "high",
                                  "Message": "this is the error message",
                                  "RawCause": "this is the raw cause",
                                  "Workaround": "this is the workaround"
                              }
            ]
    
  • only show error message, rawcause, workaround which is not nil
    [code=10001:class=database:scope=not-set:level=high], Message: database driver error, RawCause: 'tidb_retry_limit' can't be set to the value, Workaround: Please check your database config
    
    [code=48001:class=not-set:scope=not-set:level=high]
    

What is changed and how it works?

  • update ProcessError proto
  • add ErrNotSet, use for unknow error
  • better error message format

@GMHDBJD GMHDBJD added priority/normal Minor change, requires approval from ≥1 primary reviewer status/PTAL This PR is ready for review. Add this label back after committing new changes type/cherry-pick This PR is just a cherry-pick (backport) labels Jul 10, 2020
@codecov
Copy link

codecov bot commented Jul 10, 2020

Codecov Report

Merging #799 into release-1.0 will increase coverage by 0.5657%.
The diff coverage is 63.1424%.

@@                 Coverage Diff                 @@
##           release-1.0       #799        +/-   ##
===================================================
+ Coverage      57.2665%   57.8323%   +0.5657%     
===================================================
  Files              161        167         +6     
  Lines            16741      16617       -124     
===================================================
+ Hits              9587       9610        +23     
+ Misses            6206       6132        -74     
+ Partials           948        875        -73     

@lance6716
Copy link
Collaborator

LGTM

@ti-srebot
Copy link

@lance6716, Thanks for your review, however we are sorry that your vote won't be count. You are not a reviewer or committer or co-leader or leader for the related sigs:tools(slack).

@lance6716 lance6716 added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Jul 10, 2020
@WangXiangUSTC
Copy link
Contributor

please solve the conflicts

Copy link
Contributor

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

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

LGTM

@WangXiangUSTC WangXiangUSTC added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Jul 15, 2020
@GMHDBJD GMHDBJD merged commit becb317 into pingcap:release-1.0 Jul 15, 2020
@GMHDBJD GMHDBJD deleted the cherrypick771 branch July 15, 2020 06:31
@csuzhangxc csuzhangxc added needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated labels Jul 15, 2020
@csuzhangxc csuzhangxc added this to the v1.0.7 milestone Jul 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT2 Two reviewers already commented LGTM, ready for merge type/cherry-pick This PR is just a cherry-pick (backport)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants