PokeAPI / pokekotlin

Kotlin (or Java, Scala, etc) client for PokeApi
Apache License 2.0
172 stars 43 forks source link

Java Application exits if calling PokeAPI with invalid argument #66

Closed Adrian-26-Isotope closed 4 years ago

Adrian-26-Isotope commented 5 years ago

Dear Developers,

I am facing a usability problem when using pokekotlin. This is my situation: I am writing a Java application and want to cache all abilities. My first intention was to call getAbility() until I get no more abilities:

        boolean continueLoop = true;
        for (int i = 1; continueLoop; i++) {
            Ability ability = pokeApi.getAbility(i);
            if (ability == null) {
                continueLoop = false;
            }
            else {
                //do somthing
            }
        }

However as soon as i reaches a value PokeAPI returns a 404 for, my application exits with this exception:

Exception in thread "main" me.sargunvohra.lib.pokekotlin.client.ErrorResponse: (404) Not Found at me.sargunvohra.lib.pokekotlin.client.PokeApiClient.result(PokeApiClient.kt:13) at me.sargunvohra.lib.pokekotlin.client.PokeApiClient.getAbility(PokeApiClient.kt:251)

I tried to catch the exception. This didn't work. My next thought was to use the method getAbilityList(). This is working for me, but to get the actual Ability object I have to extract the id from the NamedApiResource and then do the getAbility() call with this id. This approach works but, I assume, PokeAPI is called twice for every ability. I would prefer not doing so.

I don't know nothing about kotlin and thus can't tell if this behaviour is intended or not. Also I have no idea how to prevent my application from exiting.

Wouldn't it be possible to just return null in case a HTTP call returns no result/404 error?

Regards Adrian

P.S.: Nevertheless you are doing a great job. Kudos to you!

Kronopt commented 5 years ago

By running that code you're just going over the 100 API requests per minute of pokeapi.co (https://pokeapi.co/docs/v2.html#info), as there are 293 abilities.

Please read the documentation. You shouldn't be scraping the API like that.

As for the error, that's probably the default error for a 404 http error.

sargunv commented 5 years ago

We should probably return something more informative than 404 in case of rate limiting

On Thu, Apr 18, 2019 at 16:47 Kronopt notifications@github.com wrote:

By running that code you're just going over the 100 API requests per minute of pokeapi.co (https://pokeapi.co/docs/v2.html#info), as there are 293 abilities.

Please read the documentation. You shouldn't be scraping the API like that.

As for the error, that's probably the default error for a 404 http error.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/PokeAPI/pokekotlin/issues/66#issuecomment-484725040, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKCLJK55RT2E2H3HMBKXJ3PREB7RANCNFSM4HHA5JVA .

tdmalone commented 5 years ago

Hmm yes... we should have a 429 instead of a 404. Having said that, I don't think we've actually got rate limiting enabled on the API at the moment (IIRC).

sargunv commented 5 years ago

In that case the 404 is interesting, can we confirm all the abilities are reachable?

Kronopt commented 5 years ago

I just confirmed that all 293 abilities are reachable.