MartinsOnuoha / countriesNowAPI

CountriesNow is an Open source API for retrieving geo-information for countries, including their states, cities, population, etc. 🌎
http://countriesnow.space
MIT License
179 stars 55 forks source link

API Down #90

Closed redesigned closed 2 years ago

redesigned commented 2 years ago

Not sure if this is the correct place to report this, but the API is currently down.

image

Thanks!

badakhgovind commented 2 years ago

is there any update on this? when it will get resolved?

MartinsOnuoha commented 2 years ago

Hey guys, @badakhgovind, @redesigned, thanks for reporting. Currently we have high usage on the API, we recently implemented rate limiters to reduce the number of requests from individual APIs, thanks to @TimAagaard, however that isn't a one for all solution, I would also need to increase the dyno on heroku from 1 to 2. I'm currently considering the cost implications but will do that within the week.

Meanwhile the API is back up

TimAagaard commented 2 years ago

@MartinsOnuoha I'd hold off on that.

1) I just broke your API, sorry. If you run /api/v0.1/countries/population/filter with:

{
    "year": 2020,
    "orderBy": "name"
}

The app hard crashes. I double checked locally and Node crashes. (So if you're around and awake you should restart the app)

2) If you run the same route with the values in the documentation, it runs fine

3) If you change the year to 2020 in situation 2, the route runs forever and never returns, which would cause Heroku's router to send back a 503 because your dyno is stuck processing. I don't know if its actually stuck, or taking a really long time but it's been running for over 10 minutes locally without returning - no errors from Node.

This is just one route that I checked because I thought it had the potential to be expensive.

I'm going to test all of these routes tonight and try to break them. Then I'm going to attempt to fix them.

If you don't have any objections we can start off by wrapping all the routes in a try/catch block that just returns an HTTP 500 if the catch block gets triggered. That will prevent hard crashes.

I would also recommend implementing default values for all parameters.

Beyond that, it's going to be profiling the routes for time constraints. Anything over 30s is going to cause a 503 on Heroku.

We could also implement caching to speed up repeated routes if you don't already have that.

MartinsOnuoha commented 2 years ago

Hey Tim, @TimAagaard , many thanks for breaking the application and documenting your finding, from the logs on paper trail I could tell that a single request was what keeps breaking the application although I hadn’t gotten enough time to look into it yet because of work. This is a really helpful place to to start, the POST /cities endpoint that also breaks with a certain payload, I would have to investigate this as well.

I’m totally okay with us going this route and fixing any route with this possible failure. Generally we do need to handle long running requests and cache their responses too, this was another thing I hadn’t gotten around to doing yet (although I was particular about making external calls to 3rd party data source on application startup, so it’s not repeated for every request).

I would also open an issue to move the application to Typescript, to make development even easier. Although this would be long-term and slow paced until we have a working TS version of the application.

MartinsOnuoha commented 2 years ago

I’ll close this issue again and I’ve opened a new one with your comment quoted. #91