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

Marika doesn't throw an error if Jikan responds with an error #6

Closed
YoshiWalsh opened this issue Apr 4, 2023 · 1 comment · Fixed by #7
Closed

Marika doesn't throw an error if Jikan responds with an error #6

YoshiWalsh opened this issue Apr 4, 2023 · 1 comment · Fixed by #7

Comments

@YoshiWalsh
Copy link
Contributor

YoshiWalsh commented Apr 4, 2023

The promise resolved with

{
    "status":408,
    "type":"TimeoutException",
    "message":"Request to MyAnimeList.net timed out (10 seconds)",
    "error":"Idle timeout reached for \"https://myanimelist.net/anime/2356/_/characters\"."
}

The expected behaviour would be for the promise to be rejected.

I've only noticed this issue with getAnimeCharacters.

Also, this method (and a few others, such as getAnimeStaff return an object with a data property, whereas most methods (such as getAnimeFullById) unwrap the data property. I'm not sure why this is the case, as Jikan uses the data property in all responses.

I mention this because I'm not sure if this issue is unique to getAnimeCharacters, if it applies only to methods that include the data nesting, or if it applies to all methods.

EDIT: In my logs I can see I've experienced this issue 6 times for getAnimeCharacters. I have not yet experienced it for getAnimeStats or getAnimeFullById which are the only two other methods I'm using. It's possible that I simply haven't seen a timeout for these methods, but that seems unlikely because getAnimeStats times out pretty frequently (jikan-me/jikan-rest#269).

EDIT2: Turns out the same issue is (kinda) happening for getAnimeStats too. But because this function unwraps the data property, the promise resolves with undefined instead of the error object. I have 21 occurrences of this in my logs. (Out of 816 times that I called getAnimeStats)

I'll submit a PR to fix this some time within the next few days.

@LuckyYam Is it okay if I make a breaking change to make the response format consistent, always unwrapping data?

@YoshiWalsh YoshiWalsh changed the title anime.getAnimeCharacters sometimes resolves promise with error response Marika doesn't throw an error if Jikan responds with an error Apr 4, 2023
YoshiWalsh added a commit to YoshiWalsh/wrongopinions that referenced this issue Apr 4, 2023
@LuckyYam
Copy link
Owner

LuckyYam commented Apr 5, 2023

A PR would be appreciated

@LuckyYam LuckyYam mentioned this issue Sep 20, 2023
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

Successfully merging a pull request may close this issue.

2 participants