Introduce better error handling for unknown locations

[meezaan]

As /play and /calendar are now rendering on the server side, we're logging some bad location errors for locations we can't geolocate.

We need a pretty and graceful way to handle these 400 errors from the API.

A try and catch will do, but the app routes file needs cleaning up and this is an opportunity to get a handle on some of that code.

[papasmile]

Seems that only applies to routes under /play etc with params. In any case perhaps just showing an error message on location field (similar to #14) would be good?

[papasmile]

Also, what are /a/b routes for? They don't seem to be used... can those be deleted or maybe need an enhancement issue?

https://github.com/islamic-network/aladhan.com/blob/f465f37438a07f4687384c3e8f33fbdf10d97223/src/routes.php#L420

[meezaan]

@papasmile They are to handle legacy calls from when we used to calculate co-ordinates from a local database rather than the Google Maps API. That's why the are 301s. I can check in the logs to see if they actually get any traffic and if not, we can just delete them.

[meezaan]

Effectively, https://aladhan.com/play/somecity/a%20country should throw a 404 (not a 400 as returned by the API) and should probably be a prettier page.

[papasmile]

Sounds good.

Should we log an issue over on api.aladhan to change that 400 to a 404?

https://github.com/islamic-network/api.aladhan.com/blob/fcaa0630e0c110978ecbae2095303c39d5cfdcd3/routes/timings/v1/prayerTimes.php#L613

[meezaan]

@papasmile I need to think about that. RESTfully, for the API 404 is not the right message because the city and country are query parameters, not RESTful resources. That's why i set it to a 400 in the first place.

For the app, it is the correct message because literally that URL does not exist.

[papasmile]

Salam, @meezaan, sure. I think regardless of whether we are loading an app or a json file it would be more appropriate to use query parameters than path parameters. That is, our application is located at /play. But it is configurable based on input parameters (for example, either by post or get).

That would require merging present routing code for /play and /play/city/country but seems like it would make sense... what do you think?

[meezaan]

Alaykum Salaam @papasmile - apologies for the delay.

The URLs are different on purpose - one needs semantically correct URLs because it is a webpage - the other is effectively configuration. For the API, the timestamp/data is the path parameter because RESTfully that is the resource we are getting times for.

For play, our resource itself is the location. But this issue should be easily dealt with - I think semantically correct thing to do would be to capture the server side 400 the API returns and convert it into a 404 page for the user in the app.