gousiosg
Need better api error handling
def api_request_raw(url, media_type = '') in api_client sometimes returns nil and sometimes throws an exception. This ripples up to pretty much all of the retrieve* and ensure* methods and beyond. Some of these built to handle nil and others are not. We are seeing many secondary exceptions as a result of nil return values. This typically happens when a 403 Forbidden is returned from GitHub. While I suspect that is a throttling related problem (subsequent calls appear to work), it is completely realistic that this happen as there are a raft of different REST call status that will cause nil to be returned.
I started by putting .nil? checks in the appropriate places but:
- there are quite a few places
- makes the code look yucky
- generally these just exit the method
The bonus of checking in the caller is that you can provide a somewhat more targeted error. Rather then "could not retrieve user XXX", the code could give "Failed ensure_commit because user XXX could not be retrieved"
If an exception is to be thrown, there are a number of "send() loops" that will need to be augmented with rescues.
I'm happy to help make the related changes but need to know
- exceptions or nil? checks?
- fix/change/remove existing nil? checks if exceptions are the chosen path?
