Skip to content

Improve error messages in exceptions from Artifactory #200

@BrukerJWD

Description

@BrukerJWD

I was playing around with permissions and got an error 400. This is the output of the exception message:

requests.exceptions.HTTPError: 400 Client Error:  for url: https://myartifactory/artifactory/api/v2/security/permissions/all-test

400 is not much information about what went wrong, so I dug inside your code and the request library. Finally, I printed the correct variable and found that Artifactory returned this content:

{
  "errors" : [ {
    "status" : 400,
    "message" : "The user: \'username\'\' has admin privileges, and cannot be added to a Permission Target."
  } ]
}

This was a helpful error message :) (Although I disagree on the decision behind it).
Ignoring the error message returned by Artifactory in JSON seems not like a good idea to me. It would be better if the error message is also put into the exception which is thrown.

Currently I am wrapping calls to your library with my own exception handler to achieve this:

def get_errors_from_json_response(response: requests.Response) -> Optional[List[str]]:
    response_str: bytes = response.content
    try:
        response_dict: dict = json.loads(response_str)
    except:
        return None
    if not response_dict or not "errors" in response_dict:
        return None
    return [ f"{error['status']}: {error['message']}" for error in response_dict["errors"] ]

@contextlib.contextmanager
def exception_message(msg):
    # from https://stackoverflow.com/questions/17677680/how-can-i-add-context-to-an-exception-in-python
    def raise_with_new_msg(ex: Exception, new_msg: str):
        ex.args = (new_msg,) + ex.args[1:]
        raise
    def raise_and_add_message(ex: Exception, new_msg):
        msg = '{}: {}'.format(new_msg, ex.args[0]) if ex.args else str(new_msg)
        raise_with_new_msg(ex, msg)

    try:
        yield
    except HTTPError as ex:
        errors = get_errors_from_json_response(ex.response)
        if not errors:
            raise_and_add_message(msg)
        raise_with_new_msg(ex, f"{msg}: {'; '.join(errors)}")
    except Exception as ex:
        raise_and_add_message(ex, msg)

Of course, catching exceptions, modifying and re-throwing them is not the best idea. But otherwise I do not get the response object.
On the other hand, you could implement something like raise_for_status() yourself in _generic_http_method_request(), check if the response contains some json with errors and put that into the message, and otherwise use the default implementation:

    response: Response = http_method(
        f"{self._artifactory.url}/{route}",
        auth=self._auth,
        **kwargs,
        verify=self._verify,
        cert=self._cert,
    )
    if raise_for_status and response.status_code >= 400:
        errors = get_errors_from_json_response(response)
        if errors:
            raise ArtifactoryError("; ".join(errors))
        else:
            response.raise_for_status()

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions