Conversation
SpiceQL/src/api.cpp
Outdated
| } | ||
| } catch (exception &e) { | ||
| checkNaifErrors(); | ||
| SPDLOG_DEBUG("Could not get a successful response from the web API."); |
There was a problem hiding this comment.
So in the case of an exception, I think we should throw another excpetion, one of the std exceptions, this way the rest of the function doesn't run. I imagine that would be the preffered behavior.
Also I hate to say this since you already did it, but would it not be better to put the throw catch in spiceAPIQuery so you dont have to change all the functiuons when this logic changes?
There was a problem hiding this comment.
Yeah, that makes sense. The only reason I added the throw catch to each function is because a REST API call can still return a valid JSON object like
{"statusCode":500,"body":{"error":"std::exception: Error Occured:SPICE(NOFRAMECONNECT) ..."}}
and it fails in the specific conversion for each function like json2DFloatArrayTo2DVector() in getTargetStates.
So it seems a better catch would be to check the status code in spiceAPIQuery() and if it's a 500, then to throw an exception.
There was a problem hiding this comment.
Then check the error code, and make sure it is coverted to a C++ exception by checking it then throwing if statusCode =/= 200.
There was a problem hiding this comment.
Added another check to see if JSON is valid and two more within each restincurl call.
Changes made:
SPICEQL_REST_URLif setSPICEQL_DEV_DBinstead ofSSPICE_DEBUGSSPICE_DEBUGwithSPICEQL_DEV_DBand updated docs to affirm thatSPICEQL_DEV_DBmust be set to "True" to be used (not like previously where it could be set to anything)Licensing
This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words: