Problem/Motivation:
Right now if such an entity_type is specified which does not exists, in case of 'load_by_id' it produces a Drupal error and in case of 'load_by_uuid' it results in WSOD.

Proposed Solution:
Do a check whether the specified entity_type exists before trying to load the entity. In case that entity_type is not found, do not apply the filter and leave the placeholder text intact. (Similar behavior when image load fails - alt text, in our case. The placeholder text that may have been inserted inside the entity div).

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Sound good. I think it should just be easy as wrapping it as a try-catch statement around the entity loading and rendering.

cs_shadow’s picture

cs_shadow commented
Status: Active » Needs review

Pull Request for this issue: https://github.com/drupal-media/entity_embed/pull/10

cs_shadow’s picture

cs_shadow commented

Here, I've used if-else if that's fine. It'll be less expensive than try-catch IMO. Error will be detected quite early in this case. Let me know if we need to change this to try-catch because I'm not sure what's more appropriate in this case.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

I think it would better to use try/catch since there could be other errors when loading or rendering the entity. It feels more proper developer-wise as well.

cs_shadow’s picture

cs_shadow commented

New PR: https://github.com/drupal-media/entity_embed/pull/12

Previous one was messed up, so created a new one.

slashrsm’s picture

slashrsm commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.