Skip to content

[vtadmin] propagate error contexts#7642

Merged
rohit-nayak-ps merged 4 commits intovitessio:masterfrom
tinyspeck:am_vtadmin_error_contexts
Mar 9, 2021
Merged

[vtadmin] propagate error contexts#7642
rohit-nayak-ps merged 4 commits intovitessio:masterfrom
tinyspeck:am_vtadmin_error_contexts

Conversation

@ajm188
Copy link
Contributor

@ajm188 ajm188 commented Mar 8, 2021

Description

This PR wraps a bunch of places where errors happen with additional context around what failed. Because debugging should be less hard.

Related Issue(s)

Checklist

  • Should this PR be backported? no
  • Tests were added or are not required n/a
  • Documentation was added or is not required n/a

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build/CI
  • VTAdmin

Andrew Mason added 3 commits March 8, 2021 14:02
…ditions

Signed-off-by: Andrew Mason <amason@slack-corp.com>
…ring

Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 changed the title Am vtadmin error contexts [vtadmin] propagate error contexts Mar 8, 2021
Copy link
Contributor

@doeg doeg left a comment

Choose a reason for hiding this comment

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

Thank you!


if err != nil {
return nil, err
return nil, fmt.Errorf("cannot find serving, non-MASTER tablet in keyspace=%s: %w", req.Keyspace, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about non-PRIMARY?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, i was trying to stick to the value of the enum (hence why it's all-caps) but screw it; let's change the english now!

Signed-off-by: Andrew Mason <amason@slack-corp.com>
@rohit-nayak-ps rohit-nayak-ps merged commit 7e1e105 into vitessio:master Mar 9, 2021
@ajm188 ajm188 deleted the am_vtadmin_error_contexts branch March 9, 2021 13:23
@askdba askdba added this to the v10.0 milestone Mar 10, 2021
@doeg doeg added the Component: VTAdmin VTadmin interface label Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: VTAdmin VTadmin interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[vtadmin] refactor error-handling to wrap errors with additional context

4 participants