Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
When a GuzzleException is caught when fetching a remote file, it is discarded, and instead the code relies on dropping through to a very generic error message stating that SFP encountered an error. This is a shame, because the Guzzle exception is likely to contain some amount of information about the reason the fetch failed.
Steps to reproduce
1. Configure a base URL for a non-existent site
2. Visit a page containing a missing image (you could remove the image from the local file system if it does exist locally.
3. Go to the watchdog log. Notice that the error message recorded says "Stage File Proxy encountered an unknown error by retrieving file ...". It does not actually mention anything about it not being able to find the site.
Proposed resolution
Log information about the exception in the catch clause using watchdog_exception().
Remaining tasks
- Create a patch
- Review
User interface changes
Error messages will be more verbose.
API changes
None.
Data model changes
None.
Release notes snippet
When encountering errors while downloading remote files, error messages in the log will now display more information about the actual problem encountered.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | stage_file_proxy-more_descriptive_error_message-3161884-3.patch | 1.25 KB | eelkeblok |
| |||
Issue fork stage_file_proxy-3161884
Show commands
Start 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 #2
eelkeblokComment #3
eelkeblokHere's a patch. It doesn't include tests (there don't seem to be any tests testing the error handling), I'm not sure if they are desired. One small nag with testing this is that the watchdog_exception function completely bypasses OO/CI logging with a logger channel, which is a downright shame, so testing it would be a bit more involved that just inspecting the logger.
Comment #4
eelkeblokComment #5
eelkeblokComment #6
tobiberlin+1 for this! I do not understand why the catch block is not used - I needed to set a breakpoint and used XDebug to find out what exactly was the problem
Comment #7
eelkeblokComment #8
RichardGaunt CreditAttribution: RichardGaunt commentedThis is fantastic, I have included this patch in my build and works as intended providing much better logging of stage file proxy exceptions.
Comment #9
eelkeblokShould we consider this RTBC then..? The code is not very exciting.
Comment #10
greggleswatchdog_exception is deprecated, right?
That could be fixed - https://www.drupal.org/node/2932520
Comment #11
eelkeblokDon't think it is, that CR is draft. Good idea, though. Didn't know that is the official "solution". Probably just as well, always had to look in the code how to format the message :)
Comment #12
gregglesGreat point, I just assumed all procedural code was gone. So it's not deprecated yet, but should be and hopefully will be soon :)
I'd be happy to commit this with that fix.
Comment #14
eelkeblokYou'd think so :) I always have a brief inner battle when an opportunity for watchdog_exception() comes along. So I'm actually glad with this guidance.
Have an MR :)
Comment #15
emek CreditAttribution: emek at Lund University commentedI have tried the patch from #13 and it works and gives me a better error message.
Comment #16
ivnish CreditAttribution: ivnish commentedIt works, thanks
Comment #18
gregglesThis is now merged/fixed. Thanks for the improvement and the reviews.