- Issue created by @joachim
- First commit to issue fork.
- Merge request !12673Resolve #3534660 "Defaultlazyplugincollectioninitializeplugin throws the" β (Closed) created by Unnamed author
- Merge request !12674#3534660 Added more specific error handling for DefaultLazyPluginCollection. β (Open) created by Unnamed author
- π―π΅Japan neptune-dc
I'm new to this whole forking thing. Sorry if it's not done correctly; it's my first time.
I've made a PR describing the error in more specific detail.
- πΊπΈUnited States smustgrave
Appears to have pipeline issues, also shouldn't we use t()
- π¬π§United Kingdom joachim
@smustgrave I don't see where it's using t()?
- π¬π§United Kingdom joachim
Thanks!
LGTM.
I've filed π LazyPluginCollection::initializePlugin() doesn't document it can throw exceptions Active for the missing docs.
- π¬π§United Kingdom nexusnovaz
Hey @joachim, I suspect @smustgrave was referring to using t() instead of sprintf(). Just in case he didn't mean this, i won't mark as needs work
- π¬π§United Kingdom joachim
Ah right, I must have not seen that version. He was right anyway - t() shouldn't be used for exception messages.
Looks like some tests need to be updated.
- First commit to issue fork.
- π¬π§United Kingdom joachim
IMO sprintf() is clearer than string interpolation.
- π―π΅Japan neptune-dc
I fixed the broken test related to our module.
I don't know how to fix the other test that is failing. It is a functional test within `ImageAdminStylesTest` and doesn't seem at all related to our thread. I'm also not sure why it is failing.
Maybe someone with more experience than I can fix the test?
- π¬π§United Kingdom joachim
> [31mβ[0m [41;37mBehat\Mink\Exception\ExpectationException: Current response status code is 500, but 404 expected.[0m
I wonder if something is catching the specific exception that we're no longer throwing.
Can you debug the test?
- π―π΅Japan neptune-dc
I don't know how to run drupal tests locally. I'm not that experienced. If I were to do anything, I would just change the expected 404 error to a 500 error. I doubt that is the responsible solution, though.
- π¬π§United Kingdom joachim
It's this code in ImageEffectFormBase:
public function buildForm(array $form, FormStateInterface $form_state, ?ImageStyleInterface $image_style = NULL, $image_effect = NULL) { $this->imageStyle = $image_style; try { $this->imageEffect = $this->prepareImageEffect($image_effect); } catch (PluginNotFoundException) { throw new NotFoundHttpException("Invalid effect id: '$image_effect'."); }
I'm not sure how we handle this, as other code elsewhere may be catching this type of exception too.
- π¬π§United Kingdom joachim
Ok, instead of:
$configuration = $this->configurations[$instance_id] ?? [];
we need to first check that $this->configurations[$instance_id] is set, and if it isn't, we throw the PluginNotFoundException.
- π―π΅Japan neptune-dc
Adding that handling increased the number of failed tests T.T