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

Broken decoding of Issue without a body (when its a PR) #233

Open
YarekTyshchenko opened this issue Dec 12, 2018 · 7 comments
Open

Broken decoding of Issue without a body (when its a PR) #233

YarekTyshchenko opened this issue Dec 12, 2018 · 7 comments

Comments

@YarekTyshchenko
Copy link
Contributor

Issues endpoints also return PRs. Body in a PR is Option, but isn't in Issue. This breaks at runtime during deserialisation.

github4s.GithubResponses$JsonParsingException: String: DownField(body)
@fedefernandez
Copy link
Contributor

Good catch @YarekTyshchenko, we have two different operations for Issues and PRs. I'm wondering what will be the right behavior here. I guess the easy one is just to make optional the body in the Issue model.

@YarekTyshchenko
Copy link
Contributor Author

Could filter out PRs from the issue endpoint, but it would cause unnecessary traffic. I'd be happy with an Optional body for now, since even in the docs they tell you to filter them yourself based on the presence of the pr urls field

@juanpedromoreno
Copy link
Member

It seems like #236 has introduced a random test failure, which I've ignored here: 24dd556, as part of #243.

We might want to re-visit this issue later on.

@JesusMtnez
Copy link
Contributor

I've done some debugging.

That test is failing because issue #1 has a body, and the test expects an empty body.

But if we use other issue with an empty body (that is a PR), like #154 , instead of receiving and empty body decoded to None, circe is decoding it to Some() 😵

More debugging is needed to know why that is happening 🤔

@YarekTyshchenko
Copy link
Contributor Author

I've seen this before, Some PRs have empty body (from json response ""), some actual null. I can't see any regularity, but i know that once the PR is edited it will never have a null body. I can only replicate this with a private repo atm

@YarekTyshchenko
Copy link
Contributor Author

I create my PRs with hub utility. It could be a factor

@JesusMtnez
Copy link
Contributor

🤔 Issues is implemented using circe generic auto, maybe we could implement the decoders/encoders making them handle those cases

@bilki bilki added the bug label Jul 11, 2019
@47erbot 47erbot removed the bug label Sep 17, 2020
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

6 participants