boto / boto

For the latest version of boto, see https://github.com/boto/boto3 -- Python interface to Amazon Web Services
http://docs.pythonboto.org/
Other
6.48k stars 2.26k forks source link

S3Connection.lookup silently eats all exceptions and returns None #2262

Open jurov opened 10 years ago

jurov commented 10 years ago

It makes debugging configuration problems much harder. In connection.py, line 540 (boto-2.28.0), function S3Connection.lookup() :

    try:
        bucket = self.get_bucket(bucket_name, validate, headers=headers)
    except:
        bucket = None
    return bucket

In my case, the original muted expression was S3ResponseError: S3ResponseError: 403 Forbidden that clearly explained where is the problem. Until this is patched properly, you can change the code into:

    try:
        bucket = self.get_bucket(bucket_name, validate, headers=headers)
    except:
        raise
    return bucket

to get at least proper error message.

It causes following problem with duplicity: https://bugs.launchpad.net/duplicity/+bug/1278529

Fingel commented 10 years ago

This one caused me quite a bit of frustration. It should be patched.

markstos commented 9 years ago

I tried the patch and can confirm it is an improvement when experienced via dejadup. Before the patch, I got a generic error that the backend hung up unexpectedly. With the patch, I get a stack dump is returned in the UI. It's ugly, but it tells me precisely where the problem is so I can move forward.

markstos commented 9 years ago

@jurov, @Fingel I took a second look at this. The docs for lookup clearly state that is designed not to throw exceptions and to return 'none' if the bucket is not found:

https://github.com/boto/boto/blob/develop/boto/s3/connection.py#L551

There is already a method, get_bucket which works in a similar way but throws an exception. So, I think duplicity should be calling get_bucket to access to the full diagnostic about what went wrong.

markstos commented 9 years ago

This reportedly fixed in duplicity v0.7.01 (2015/01/11) by using different method calls like described above, so I think this issue can be closed.

sparksis commented 8 years ago

it's been nearly a year since the last update... This really should be addressed as failing silently is not the correct solution.

saj commented 8 years ago

I think this issue can be closed

No, a catch-all except is never correct. A catch-all except will catch: exceptions triggered by ^C (SIGINT), misspelled identifiers, assertion failures, memory allocation failures, inconsistent indentation, and so and and so forth. I cannot imagine a scenario in which it would be advantageous for boto to blindly and silently gobble such failures.

Here is another downstream report that illustrates how this bug continues to frustrate users (the recurring links to duplicity are purely coincidental):

https://bugs.launchpad.net/duplicity/+bug/1618951

except Exception:, while still wrong, would be a minor improvement. A few 'priority' exceptions used by the Python runtime inherit from higher up in the class hierarchy.

markstos commented 8 years ago

No, a catch-all except is never correct

Sometimes you want a method that returns true or false and never throws. boto documents the method in question to never throw so it is working as intended. There is already a related method which does throw. The backwards-compatible solution is for calling libraries to use the methods that do throw if they want that behavior. boto is working as documented this in this case.

Yes, perhaps it would be better if the method documented to never throw never existed, but it does and some apps are using it. The best fix seems to be for the apps to use the alternate methods, which is why I think this bug report can be closed.