I was working on some customizations in my dev environment and deleted a few relationships incorrectly (not sure the exact steps to reproduce, but probably involved deleting rows directly from the database table because I was in a hurry). As a result, they still exist in some form within the system. Later, when I tried deleting one of the endpoints in one of these broken relationships, I got the following errors that prevented me from proceeding any further:

  • Notice: Trying to get property of non-object in relation_entity_delete() (line 904 of /path/to/docroot/sites/all/modules/relation/relation.module).
  • Warning: Invalid argument supplied for foreach() in relation_entity_delete() (line 904 of /path/to/docroot/code/sites/all/modules/relation/relation.module).
  • Notice: Trying to get property of non-object in relation_entity_delete() (line 911 of /path/to/docroot/code/sites/all/modules/relation/relation.module).
  • Notice: Trying to get property of non-object in relation_entity_delete() (line 912 of /path/to/docroot/code/sites/all/modules/relation/relation.module).
  • Notice: Undefined index: in relation_entity_delete() (line 912 of /path/to/docroot/code/sites/all/modules/relation/relation.module).
  • Notice: Trying to get property of non-object in relation_entity_delete() (line 912 of /path/to/docroot/code/sites/all/modules/relation/relation.module).
  • Notice: Trying to get property of non-object in relation_entity_delete() (line 913 of /path/to/docroot/code/sites/all/modules/relation/relation.module).
  • EntityMalformedException: Missing bundle property on entity of type relation. in entity_extract_ids() (line 7861 of /path/to/docroot/code/includes/common.inc).

As a result, my entity could not be deleted at all. Now while my site is still in development that's not a big deal. However, if some of these irregularities had gotten copied into production, it might have been a bigger issue.

Now I realize, this won't affect 99% of people and while it is preferable to have a clean database, that doesn't always seem practical, so I've written a patch to just skip those conditions. Perhaps we could consider writing a watchdog notice in this situation to alert the admin of data irregularities, but I'm not sure if it's necessarily import enough to risk cluttering the log with.

Either way, it's a 3 line patch so hopefully not too much of a deal to implement.

CommentFileSizeAuthor
#2 relation-avoid-error-deleting-entity-in-unclean-database-2547107-2.patch587 bytesWidgetsBurritos

Comments

davidstinemetze created an issue. See original summary.

WidgetsBurritos’s picture

WidgetsBurritos commented
StatusFileSize
new relation-avoid-error-deleting-entity-in-unclean-database-2547107-2.patch587 bytes
dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented

but probably involved deleting rows directly from the database table because I was in a hurry

Issues caused by deleting data directly from the database should not have a main priority. But thanks for the effort and the patch. But I am not sure if this is the correct approach?

However, if some of these irregularities had gotten copied into production, it might have been a bigger issue.

I hope that nobody would delete data from database directly on a production stage. An in the case of your patch it would just hide the errors and stop the function going on. I don't know if this is wanted. Maybe a maintainer should chime in here.

WidgetsBurritos’s picture

WidgetsBurritos commented

Let me clarify a little bit more. I'm presuming it's because I had just deleted stuff from the database. It may have been a bug in the code I was writing as well causing this scenario, which maybe a more likely scenario. I haven't actually been able to reproduce the error, so it might just be a minor issue and not worth worrying about. My concern is something like this happening in a development environment that then gets copied over into to production and doesn't get discovered until it's too late, because the way it existed in my system, I'm not sure if regular QA testing would have ever discovered that. I just happened to be working on that paricular component at the time.

But that's why I was proposing maybe posting some sort of error to watch dog or something. That way an admin could at least be made aware of the data inconsistency. Either way, I'm not extremely worried about it. I'm gonna clean up my data before I make it live, but it's just worth noting that this bug does exist, and it prevents your system from completing tasks. It's not just generating notices, but rather generating notices and then giving a "Unexpected error" to your users. I'd rather end-users not know something was wrong, and then let the admins deal with it later.

Perhaps the solution would be instead of this bug fix, implement a system that looks for data irregularities and cleans them up periodically. That's probably a better solution long term.

mikran’s picture

mikran commented
Status: Needs review » Closed (won't fix)

If you delete data without invoking related hooks there will be much more problems than just what's occurring here. And that will happen basically with any fieldable entity, not just relation.

I haven't actually been able to reproduce the error ...

This is won't fix unless specific and testable condition is found where the entity delete hook does not correctly delete relations or other data that's supposed to get cleaned up.

WidgetsBurritos’s picture

WidgetsBurritos commented

Fair enough.