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

error handling when kube returns an error struct missing #6

Open
clux opened this issue Nov 14, 2018 · 3 comments
Open

error handling when kube returns an error struct missing #6

clux opened this issue Nov 14, 2018 · 3 comments

Comments

@clux
Copy link
Contributor

clux commented Nov 14, 2018

One thing I noticed is that data returned from the kube api is assumed to be parseable into the struct you give it:

req.send()?.json().map_err(Error::from)

however, we have no real information on what went wrong if kube returns an error struct. We simply get a json parsing failed (usually "unexpected EOF" via serde) because the string is attempted to be coerced into a different struct to the one that contains the error.

This happens if you query for information about your own custom resources and you've failed to add the necessary rbac rules for it, but i'm sure there are many other cases.

There's currently got a fork to help me deal with this issue in my account (and to get some debug back when it's failed for now). I might be looking to submit a pr back if I stumble across a good solution to it. In the mean time, just thought this ought to be raised.

At any rate, thanks for this crate! It's allowed me to write a kube app with rust with only minor tweaks!

@ynqa
Copy link
Owner

ynqa commented Nov 15, 2018

@clux Thanks for your issues.

Ah.. right. If it happens any error on request, it's possible not to deserialize correctly for some k8s objects, such as Pod... This error handling is so not good.

There's currently got a fork to help me deal with this issue in my account (and to get some debug back when it's failed for now).

Here? kube-rs/kube@2b0397f
I also welcome to your PRs about your improvements :)

@clux
Copy link
Contributor Author

clux commented Nov 21, 2018

Yeah, that change. I added kube-rs/kube@4e3589d as well, which will try to deserialize the error.

It's not enough to just pass on reqwests error type like i do later on, because you will miss out on the important reason for why things failed. Like say on a 403, the reason often includes:

crdgroup.domain "apps" is forbidden: User "system:serviceaccount:apps:developer" cannot get crdgroups.domain in the namespace "apps"

@ynqa
Copy link
Owner

ynqa commented Nov 27, 2018

@clux Sorry for my responses delay, look good to me about your improvements!!
Essentially it should tackle handling some errors on APIs (generated) side.
I have to choose below:

  • for handling requests/responses, and their errors more flexibly, managing k8s APIs on own codes
  • for maintaining API definition easily, rely on the code generator

svend pushed a commit to svend/kubernetes-rust that referenced this issue Jul 26, 2019
third release in a day. time for a break :D
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants