department-for-transport-BODS / bods-data-extractor

A python client for downloading and extracting data from the UK Bus Open Data Service
Other
44 stars 8 forks source link

User should know why API request has returned no data #24

Closed spencer-b-318 closed 1 year ago

spencer-b-318 commented 1 year ago

Is your feature request related to a problem? Please describe. If the API request sent by TimetableExtractor returns nothing, KeyError: 'results' is raised. This error does not provide much information to the user as to why the request has returned nothing. It could be due to no data found in the database or it could be due to invalid API key. Better error handling would improve this experience.

Describe the solution you'd like A bespoke exception could be the solution, returns the API response if not '200 OK'. If API response is valid, then suggest a reason for the failure.

Describe alternatives you've considered TimetableExtractor could simply print API response but this would not cover cases where invalid search terms such as NOC have been used

Here, api_key = 0 has been passed, resulting in 403 response but this is not shown to the user

image

adamakram1 commented 1 year ago

image The status code and reason are displayed when there's an incorrect API response in a potential solution

spencer-b-318 commented 1 year ago

@adamakram1 this is better, but the error is ultimately still always going to be Key Error: 'results which is confusing.

I was thinking we create a custom exception class. This would also be able to catch a 200 status code with no data

adamakram1 commented 1 year ago

image Amended the original solution so that the error handling takes place in another class

adamakram1 commented 1 year ago

image Tested on empty datasets and when setting limit to 0

lldwork commented 1 year ago

does the custom Error class inherit from standard Error classes? Or an Exception may be more appropriate, depending on what flow you want. To avoid re-execution of code, allow user to enter a new value for any incorrect parameter where relevant.

adamakram1 commented 1 year ago

Hi, we have decided not to go ahead with the current approach with a custom error class. Instead we are raising an actual error which will help us trace back lines in debugging. The new approach can be viewed in the pull requests:

https://github.com/department-for-transport-BODS/bods-data-extractor/pull/49

adamakram1 commented 1 year ago

Leave a more specific message with regards to why we have an empty dataset, e.g NOCS