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.

Sign up for GitHub

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

Jump to bottom

Bad include can return 500 #134

Closed
stickwarslit opened this issue Oct 2, 2018 · 3 comments
Closed

Bad include can return 500 #134

stickwarslit opened this issue Oct 2, 2018 · 3 comments
Assignees
@jeregrine @snewcomer
Labels
bug

Comments

@stickwarslit
Copy link

According to the includes spec on jsonapi (1), invalid includes should only return 400.

https://github.com/jeregrine/jsonapi/blob/242499eaf42362fa0f1e6602ecc08f9baa7feaad/lib/jsonapi/query_parser.ex#L183

Looks to be due to this line. Based on whether or not the atom exists in the app, the line raises an error only on certain invalid includes
@doomspork doomspork added the bug label Oct 10, 2018
@doomspork
Copy link
Member

@MarkESDR could you provide a repo that produces this error? I think that would be easiest to work from. Do you happen to have a solution in mind?

@stickwarslit
Copy link
Author

@doomspork Here is an example repo. For solutions, how about, instead of converting the request include param to an atom to compare, converting the list of valid include params to strings. This allows us to compare the request params and valid params without creating extra atoms

@doomspork
Copy link
Member

@MarkESDR we'd love a PR to fix this if you have the time 👍

jherdman added a commit to jherdman/jsonapi that referenced this issue Jan 27, 2019
@jherdman
Fix Bad Includes Returns HTTP 500 Response
29fe1e0 
It looks like the smoking gun was in our tests this whole time.

Resolves beam-community#134
@jherdman jherdman mentioned this issue Jan 27, 2019
Merged
jherdman added a commit to jherdman/jsonapi that referenced this issue Jan 28, 2019
@jherdman
Fix Bad Includes Returns HTTP 500 Response
f04e397 
It looks like the smoking gun was in our tests this whole time.

Note that this adds some type specs at the same time in following with
the current trend in this lib of gradually adding them as we go.

Resolves beam-community#134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jeregrine jeregrine

@snewcomer snewcomer

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@doomspork @jeregrine @snewcomer @stickwarslit