I noticed that some (possibly unreachable) code in Views (added in #2904847: Replace debug() statements in Views) tries to call trigger_error() with E_WARNING instead of E_USER_WARNING, which throws ValueError: trigger_error(): Argument #2 ($error_level) must be one of E_USER_ERROR, E_USER_WARNING, E_USER_NOTICE, or E_USER_DEPRECATED. So rather than logging a warning, an error is thrown.
I'm not sure how to actually hit this code, maybe it's unreachable and could be removed?
Either patch the trigger_error() calls or remove the code if it's unreachable.
Review approach.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 3266243.patch | 2.02 KB | mfb |
Comments
Comment #4
alvarodemendoza commented@mbf The patch works. Thank you,
Comment #5
alvarodemendoza commentedComment #6
catchI think we should remove these checks, if they somehow turned out to be necessary, it seems unlikely the resulting error would be worse than the error that's already here, and we'd probably add either an exception or a better error message if we were adding back if so.
Comment #7
longwaveI traced the origin of this line back to 2008 and an extremely early version of Views:
https://git.drupalcode.org/project/views/-/commit/4fc76df7ca70395c9795a4...
It's not even clear then why the row plugin might be missing; I agree with @catch that we should just remove these checks instead.
Comment #8
quietone commentedThis was discussed in bugsmash, adding tag.
Comment #9
mfbI assume you mean remove the checks and don't
return [];here, not just remove the trigger_error() calls?Btw, to further clarify what I said in the issue summary, it seemed like if a plugin is missing what actually happens is you get a PluginNotFoundException, so that's why I thought this code might not be reachable.
Comment #10
longwaveYes, remove the whole
ifstatement; if it crashes later so be it, but as you say I suspect this will be caught by the plugin manager anyway and this is probably just dead code.Comment #11
mfbComment #12
longwaveThank you, this looks good to me.
Comment #14
catchCommitted/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x, thanks!