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

Send 404 when the resource or author is not found #48

Open
itaquito opened this issue Jun 18, 2021 · 6 comments · May be fixed by #50
Open

Send 404 when the resource or author is not found #48

itaquito opened this issue Jun 18, 2021 · 6 comments · May be fixed by #50
Assignees
Labels
awaiting-pr bug Something isn't working

Comments

@itaquito
Copy link

Hello!

Right now when you search for a resource/author that doesn't exist you get an interesting response:

{
  "id": null,
  "title": null,
  "tag": null,
  "current_version": null,
  "native_minecraft_version": null,
  "supported_minecraft_versions": null,
  "icon_link": "https://www.spigotmc.org/styles/default/xenresource/resource_icon.png",
  "author": {
    "id": null,
    "username": null
  },
  "premium": {
    "price": null,
    "currency": null
  },
  "stats": {
    "downloads": null,
    "updates": null,
    "reviews": {
      "unique": null,
      "total": null
    },
    "rating": null
  },
  "description": null
}
{
  "id": null,
  "username": null,
  "resource_count": null,
  "identities": {},
  "avatar": "https://www.spigotmc.org/styles/spigot/xenforo/avatars/avatar_male_l.png"
}

My suggestion is just adding a 404 error when this happens:

{"code":404,"message":"Resource not found."}

By doing this way, we (devs that use the API) can easily catch the error rather than checking if a value is null.

Also.... Thanks for this amazing tool.

@jacobsandersen
Copy link
Collaborator

jacobsandersen commented Jun 18, 2021

Oh, wow. This is a regression. What routes does this happen on?

@jacobsandersen jacobsandersen added the bug Something isn't working label Jun 18, 2021
@jacobsandersen
Copy link
Collaborator

Reproduced on getAuthor and getResource but not findAuthor. Thank you I will submit a patch tomorrow.

@jacobsandersen jacobsandersen self-assigned this Jun 18, 2021
@itaquito
Copy link
Author

Oh! nvm, It is a bug! You were faster than me xP

Thanks again.

@itaquito
Copy link
Author

I found a discrepancy in the 404 errors:
When you navigate thru the pages of getResourcesByAuthor, listResources or getResourceUpdates you will get an empty array when you get to the last page. However, when you go to a negative number page you will get {"code":404,"message":"Nothing was found for that request."}.

Shouldn't both responses be the same?

@jacobsandersen
Copy link
Collaborator

I found a discrepancy in the 404 errors:
When you navigate thru the pages of getResourcesByAuthor, listResources or getResourceUpdates you will get an empty array when you get to the last page. However, when you go to a negative number page you will get {"code":404,"message":"Nothing was found for that request."}.

Shouldn't both responses be the same?

Fair point, I'll address this as well tomorrow. Thank you

jacobsandersen added a commit that referenced this issue Jun 19, 2021
…pull up resources by user/updates by resource; also check if listResources has any results and if the array is empty (i.e. no more results) then send a 4

04 instead of a plain empty array off to infinity; closes #48.
@jacobsandersen jacobsandersen linked a pull request Jun 19, 2021 that will close this issue
@jacobsandersen
Copy link
Collaborator

Took me one extra day - sorry. Had stuff going on yesterday.

jacobsandersen added a commit that referenced this issue Nov 4, 2022
…pull up resources by user/updates by resource; also check if listResources has any results and if the array is empty (i.e. no more results) then send a 404 instead of a plain empty array off to infinity; closes #48.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-pr bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@jacobsandersen @itaquito and others