nornir_netbox icon indicating copy to clipboard operation
nornir_netbox copied to clipboard

Return NetBox error on invalid request

Open nahun opened this issue 2 years ago • 2 comments

Right now if NetBox returns an error, the plugin returns a generic error message that isn't very helpful:

Failed to get data from NetBox instance {YOUR-INSTANCE}

It'd be nice if the plugin returned the error that was given by NetBox here: https://github.com/wvandeun/nornir_netbox/blob/860d2f80e3fd2b2989c33044c560decdd2685cf1/nornir_netbox/plugins/inventory/netbox.py#L387-L390

Maybe something like this:

if not r.status_code == 200:
    raise ValueError(
        f"Failed to get data from NetBox instance {self.nb_url}", r.json()
    )

I know that would return a tuple to the user and maybe r.json() doesn't always return data so this probably isn't the best way. But just wanted to give an idea.

My use case is if the user gives an invalid filter. NetBox will return a 400 error like this:

{'site': ['Select a valid choice. TEST is not one of the available choices.']}

So it'd be great to give that feedback to the user.

I can submit a PR if we determine the best method.

nahun avatar May 11 '22 01:05 nahun

Good point. Not sure if we can rely on the fact that r.json() will always succeed. AFAIK it will raise an exception if the response body is not valid JSON, for example a plain string.

We could try to append the error message, if the returned Content-Type is "application/json", but I would just dump the raw data instead of first de-serializing it.

wvandeun avatar May 11 '22 22:05 wvandeun

I'd say two options:

Simple:

if not r.status_code == 200:
    raise ValueError(
        f"Error retrieving data from NetBox: {r.text}"
    )

Complex:

if not r.status_code == 200:
    try:
        raise ValueError(r.json())
    except requests.exceptions.JSONDecodeError:
        raise ValueError(
            f"Error retrieving data from NetBox: {r.text}"
        )

With complex, if the user wants, they could then access the de-serialized object:

try:
    nr = InitNornir(...)
except ValueError as e:
    print(e.args[0])

Seems odd, so probably go with simple?

nahun avatar May 11 '22 22:05 nahun