Make exception handling more Pythonic.

[matecsaj]

The way exceptions are thrown to users and the information they contain could be more precise, and the underlying code cleaner.

[matecsaj]

Citing Andrew M. from here.

I think handling exceptions in Python is usually a bad thing. The exception is when you are expecting that something might fail e.g. a network request or similar. But for stuff that should work, exception handling that doesn't actually handle anything (e.g. just wraps one error in another) just adds a layer of complexity when something is breaking. It's much easier to debug using something when you get the actual error not just a 'this thing failed somewhere' error and no detailed traceback. You also need to be extremely selective about what happens between 'try' and 'except'. For example, around line 804 in token.py:

        body = CreateSigningKeyRequest(signing_key_cipher='ED25519')
        try:
            key = api.developer_key_management_create_signing_key(body=body)
        except Error as e:
            raise Error(
                    number=96024,
                    reason='Failed to create key pair',
                    detail='Failed to create key pair: {}'.format(e)
                )
        else:
            self._save_key(key)

If something in api.developer_key_management_create_signing_key raises a meaningful 'Error', this gets overwritten with a new 'Error' message and code. You do get the error pasted into the 'detail' part, but you lose the traceback and the original exception (except its str representation) which may make debugging harder. On the other hand the bit of code raising the Error inside the swagger code is probably only doing something similar anyway...

Something that might assist: you can raise one exception 'from' another exception; that way I think you keep the traceback (you get two tracebacks, I think). https://docs.python.org/3/tutorial/errors.html#exception-chaining

Finally, it doesn't really matter but if you are raising an error in all the 'Except' clauses of try/except/else then the 'else' is redundant. For example, in the second test I had:

        try:
            result = self._api.sell_finances_get_transaction_summary(filter="transactionStatus:{PAYOUT}")
        except Error as error:
            self.fail(f'Error {error.number} is {error.reason}  {error.detail}.\n')
        else:
            self.assertTrue('credit_count' in result)

where the else clause is redundant, but I probably left it in as it's effectively the conclusion of the first part of the test and it's then obvious that it is for the 'succeeds' case? Probably I should have dropped it. But in your modification, the whole of the rest of the test is then moved into the else clause which just increases the indentation (and avoiding excessive indentation/nesting is generally a good thing).

5) I'm not sure why, when a unit test case fails during creation, you don't just allow the error to be raised in the setupClass (which will halt the test case entirely) but instead fail in setup? There may be a reason for this :) I also think it's fine for tests to raise an error rather than failing - to me that's a meaningful outcome that in some cases is distinct from a simple fail e.g. 'pass' the test produced the expected result, 'fail' the test did not produce the expected result, 'error' the test failed in an unexpected way...

6) Obviously you have your error codes. The alternative is that you can make a single 'eBayRestError' class derived from Exception and the make further exception classes like 'eBayRestKeyError' or whatever derived from that. Then a third-party application only has to catch 'eBayRestError' and they will also catch all of the subclasses, but they can also catch the more specific errors if they want to.

[matecsaj]

Relevant history.

Initially, I used the custom Error class to encapsulate all possible errors from Swagger API calls. To minimize the number of exception handlers a library user must have, I extended the error class to cover all errors. Should a user want to distinguish been errors, the error number is unique and will never change.