Return Metadata instead of raising TypeError on invalid requests

ctdunc commented 6 months ago

Code Sample, a copy-pastable example if possible

from blp import blp

sec_ids = ["US5949181045", "BMG0684D3054"] #BMG0684D3054 changed domiciles, but index won't update until monthend
with blp.BlpQuery("****") as bbg_conn:
    bbg_data = bbg_conn.bdp(sec_ids, ["dvd_ex_dt", "dvd_pay_dt"])

    print(bbg_data)
>>> ... 
>>> TypeError: Response for 'US5949181045' contains securityError {'securityError': {'source': '24929:rsfrdsv c2', 'code': 43, 'category': 'BAD_SEC', 'message': 'Unknown/Invalid Security  [nid:24929]', 'subcategory': 'INVALID_SECURITY'}}

Problem description

We have a workflow which sometimes places expired security identifiers into blp requests. Generic TypeErrors do not return enough information for us to handle them well without writing logic to parse error messages across the codebase.

Expected Output

It would be great if it returned this securityError as a dict or object so that we may handle it gracefully by dropping the security & retrying the request. On a larger scale, it would be an improvement to allow for more graceful error handling by returning/raising error objects with more metadata. Happy to work on a PR with changes if this is something you're interested in implementing.

ctdunc commented 6 months ago

There is a workaround for this -- ignoring errors with blp.BlpQuery("***", parser=blp.BlpParser(raise_security_errors=False)). It would still be good to have more digestible information about the error returned to the user.

matthewgilbert commented 6 months ago

Hi @ctdunc , I'm happy to accept a PR on this, open to suggestions. My initial thoughts are we would want to introduce a single error type like the one below and replace a subset of the TypeErrors with it.

class BlpParseError(Exception):
    ...

Admitted there are a few different error types here,

One is around the response type being unknown (likely because some functionality of the api has not yet been wrapped)
another is around response messages containing errors
Another is around the user using the wrong endpoint, e.g. bulk reference data vs reference data

I think treating these separately is maybe a bit of over engineering though, but happy to hear if you have a use case for this.

On your last point and embedding meta data in the error, what were you thinking? Maybe something like

class BlpParseError(Exception):
    def __str__(self):
        return f"Parsing error occurred with request {self.args[0]} and response {self.args[1]}"

and then the request and the response can be passed into the exception as args, which will also allow them to be accesses later by user specific error handling code.

ctdunc commented 6 months ago

Hi Matthew---thanks for responding! For our use case at least, it would be useful to have something like

class BlpParseError(Exception):
    def __str__(self):
        return f"Parsing error occurred with request {self.args[0]} and response {self.args[1]}"

    @property
    def error_response(self):
        #obviously this would return the actual error message & not just my copy/pasted output
        return {{'securityError': {'source': '****', 'code': 43, 'category': 'BAD_SEC', 'message': 'Unknown/Invalid Security  [nid:24978]', 'subcategory': 'INVALID_SECURITY'}}

where we can handle the response without looking at the string representation of the error.

As for the different error types, I actually do think it may be a good idea to subclass different error types here. E.g. for unrecognized/unwrapped fields, we may want to raise a NotImplementedError. In the situation of invalid security ids/other bad arguments, a ValueError might be more descriptive, since we did submit a query of valid (string) type, but with a bad value, & raise a TypeError for bulk vs reference data, since the logic in handling these may be different. For example, we use BLP to run queries behind the scenes in a flask app & would handle a NotImplementedError by removing that logic from the app altogether, vs a ValueError by requiring some different input from the end user, or dropping invalid values from our bulk queries & alerting the user.

matthewgilbert commented 6 months ago

Okay sure that sounds reasonable. If you want to submit a PR implementing this functionality I'd be happy to review it.

RushiVichare commented 2 months ago

Hey I am also facing kinda same errors while running my script.....Right now Its I think shows from the Class BlpParser . The error shows as something like this it's basically on my organization systems given as this : 1716370993854501308585540373386 Also to check does wrapper only supports to who owns terminals as Bloomberg anywhere or is it can be used for who doesn't own personal Bloomberg??

matthewgilbert / blp