-
Notifications
You must be signed in to change notification settings - Fork 101
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
Error handling for registry #499
Error handling for registry #499
Conversation
Signed-off-by: Jougan-0 <prasantmishra2018@gmail.com>
Signed-off-by: Jougan-0 <prasantmishra2018@gmail.com>
Signed-off-by: Jougan-0 <prasantmishra2018@gmail.com>
…0/meshkit into errorHandlingForRegistry
models/meshmodel/registry/error.go
Outdated
) | ||
|
||
func ErrUnknownHost(err error) error { | ||
return errors.New(ErrUnknownHostCode, errors.Alert, []string{"host is not supported"}, []string{err.Error()}, []string{"The component's host is not supported by the version of server you are running"}, []string{"Try upgrading to latest available version"}) | ||
} | ||
func ErrUnknownHostInMap() error { | ||
return errors.New(ErrUnknownHostCode, errors.Alert, []string{"Registrant has no error or it is not supported or unknown."}, nil, []string{"The host registering the entites has no errors with it's entites or is unknown."}, []string{"Validate the registrant name again or check /server/cmd/registery_attempts.json for futher details"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Registrant has no error...." Hmm. What?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The part needs rephrasing did it .
The error is resulted when we access the map of the attempts variable with different registrants and it does not contain that registrant so it shouldn't do anything but vihas mentioned that it might panic so the error is for that. I haven't seen this edge case ever happened so I think this is void but still good to have a error if it happens.
Signed-off-by: Jougan-0 <prasantmishra2018@gmail.com>
Signed-off-by: Shlok Mishra <99207534+Jougan-0@users.noreply.github.com>
Signed-off-by: Jougan-0 <prasantmishra2018@gmail.com>
While testing the fix of panic error the error code was being shown as None event though it existed. New commit should fix it. |
Should fix this issue too. |
errors/errors.go
Outdated
defer func() { | ||
if r := recover(); r != nil { | ||
severity = None | ||
fmt.Println("Recovered from panic:", r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know that this is necessary and, at first glance, it seems redundant or not insightful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for mentioning it was redundant, by mistake I pushed defer on top the assertion logic .
Reason behind using anonymous function is the assertion logic, which I used was giving NONE
on meshkit errors that existed when I released my local meshkit release and used it in go.mod but working fine for local meshkit so new logic is that if we get a panic rather than panic and shutting down the process we catch the panic and returns old value.
Signed-off-by: Jougan-0 <prasantmishra2018@gmail.com>
Signed-off-by: Jougan-0 <prasantmishra2018@gmail.com>
@Jougan-0 will you reconcile the two different error codes for |
Signed-off-by: Shlok Mishra <99207534+Jougan-0@users.noreply.github.com>
Description
This PR fixes #
Notes for Reviewers
Signed commits