Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return NetBox error on invalid request #41

Open
nahun opened this issue May 11, 2022 · 2 comments
Open

Return NetBox error on invalid request #41

nahun opened this issue May 11, 2022 · 2 comments

Comments

@nahun
Copy link

nahun commented May 11, 2022

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:

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

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.

@wvandeun
Copy link
Owner

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.

@nahun
Copy link
Author

nahun commented May 11, 2022

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants