It's pretty hard to figure out what's going on under the hood during a file operation error. AWS client throws very handy exceptions, but neither of them is catched.
Just make any error, check the error log, only the core file system errors are logged, nothing AWS specific.
Catch and log AWS exceptions.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3293269-better-logging.patch | 5.94 KB | luizsgpetri |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
szantog commentedHmm, I don't know why are the tests failing.. Pls review.
Comment #4
cmlaraComments added.
Main reason for tests failure is that a Guzzle error is now being allowed to make it through without being captured. Maybe that is a root fault inside of AWS (would have to look at the stack) however honestly we really do want to make sure we capture every failure at this point and report it as a failed copy to avoid possible WSOD's so at least a general \Exception remaining is still useful. Drupal core file_system service uses error suppression when calling methods so catching \Exception is not out of place in that regard.
Comment #5
cmlaraForgot to change status.
Comment #6
hugronaphor commentedThis seems to have been merged already?
Comment #7
cmlaraNot yet.
This needs a rebase on top of latest HEAD and the open comments to be addressed.
Comment #8
luizsgpetri commentedHi, I rerolled the patch up to 8.3.3 version.
I wasn't able to create an interdiff file because the previous patch doesn't apply anymore.
I've found out that In some cases the getAwsErrorMessage() method is returns an empty string. Like when we try to use an nonexistent bucket.
So I've added a fallback to it.
All the comments in the previous patch are now resolved but the one related with providing a fallback to the logger parameter. It mentioned that this would be needed because we were in beta, which doesn't seems to be the case anymore.
Comment #9
luizsgpetri commentedComment #10
cmlaraIt’s a “once a BETA release is made, BC measure should be done” which especially applies to Stable since we didn’t mark the class internal it may be being extended.
The simple fallback for this is, when the parameter is null fallback to calling \Drupal::service() to get the logger and raise a deprecation notice.
Comment #12
cmlaraCommitted to 8.x-3.x needs port to 4.x