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

API Down #90

Closed
redesigned opened this issue Mar 7, 2022 · 5 comments
Closed

API Down #90

redesigned opened this issue Mar 7, 2022 · 5 comments

Comments

@redesigned
Copy link

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

Thanks!

@badakhgovind
Copy link

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

@MartinsOnuoha
Copy link
Owner

MartinsOnuoha commented Mar 8, 2022

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
Copy link
Contributor

@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)

  1. If you run the same route with the values in the documentation, it runs fine

  2. 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 MartinsOnuoha reopened this Mar 8, 2022
@MartinsOnuoha
Copy link
Owner

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
Copy link
Owner

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

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

No branches or pull requests

4 participants