When enabling commerce and its suit of modules after having it disabled, a PHP Fatal Error is provoked.
I encountered this when switching to the ctools dev branch and thus having to disable ctools and the modules that depend on it.

WD php: FieldException: Attempt to create field name commerce_customer_address [error]
which already exists, although it is inactive. in field_create_field() (line 81 of /dummysite/modules/field/field.crud.inc).

Can someone confirm this behavior?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cjoy’s picture

Priority: Normal » Major
rszrama’s picture

Priority: Major » Normal

I don't understand why you thought you needed to disable all the Commerce modules just because of the Ctools change. You can change a module branch at will without worrying about it upsetting other modules, but disabling a whole suite of modules that defines both entities and fields in use on the site is bound to cause problems. The particular issue you're seeing appears to be with our default field creation code; it normally expects the fields to have been deleted on uninstallation and would attempt to recreate them, but somehow yours just became disabled because you didn't fully delete the modules. That appears to not be picked up on by the check we use to see if a field already exists.

It's really a world of hurt to go down the disable / re-enable route. D7's field support just requires a lot of hand holding to do funky things like this (which is why we had to make special accommodation in our modules for deleting fields on module uninstallation). Perhaps you should just open your field_config table and re-enable these disabled fields then re-enable the modules. Does that resolve it for you?

royerd’s picture

I ran into the same problems. I had to disable some modules trying to troubleshoot an error, and before long, I'm screwed here. I'm not sure if I should start the site over with a new database or perhaps i can try to reenable fields in the database.

----> OK, holding open the database on the field_config table and changing the "active" field from 0 to 1 eventually fixed the issue for me. When I resinstalled the module, those fields appeared again with zeros and i had to change them back to a 1 or active state. Through careful attention, two windows open at the same time (my phpmyadmin and the module install window in drupal) and eventually it all worked.

Thanks for that tip Ryan.

Dan

rszrama’s picture

Ok, cool. It's totally lame, I know, but at least there's a temporary workaround. I suppose we can update our field configuration functions to look for disabled fields first, but man it sure would be nice if the Field API would detect disabled fields earlier. : P

rszrama’s picture

Version: 7.x-1.1 » 7.x-1.x-dev

Chatted this over with Damien, and he seemed to think core could possibly have been responsible for re-enabling fields that were disabled due to a defining module getting disabled. That's the next place to investigate. It is pretty silly that the field module should be responsible for re-enabling something that core was responsible for disabling.

amateescu’s picture

Title: re-enabling commerce causes PHP Fatal Error » Re-enabling Commerce causes a PHP Fatal Error
Component: Customer » Developer experience
FileSize
13.35 KB

After talking with Bojan about this, the agreed approach was to move our field creation stuff from hook_enable() and hook_modules_enabled() to their *_install() counterparts.

I'm also tempted to move (and underscore) the functions that actually create our fields over in .install files in order to lose some code weight, but that's probably an API change. Should we do that and mark the existing ones as deprecated for 1.5?

rszrama’s picture

The problem with a wholesale move like this is the scenario where a module is disabled but not uninstalled when a new module gets installed. We need to be re-checking, for example, when the Customer module is re-enabled after being disabled but not uninstalled that there are no new customer profile types requiring addressfields. Perhaps you accommodated this in the patch and I missed it. : ?

I'd prefer to leave the API as is in a point release to avoid breaking any other modules depending on the functions. Can we not just accommodate checking to ensure fields don't need to be re-enabled like we check if they've already been created?

bojanz’s picture

We can all agree that disabled modules are a broken concept that can only cause you pain, and that really shows with a complex project like Commerce.

However, Commerce makes it worse by changing the "contract" and making disabled modules broken in a different way.
The obvious reason is the age of the code. Back when Commerce was written there was no "create fields on install" rule for D7 contrib.
That has changed. Modules are never supposed to use hook_enable() to create fields. This is a frequent WTF that is mentioned on IRC by Commerce and non-Commerce people, as well as in our internal conversations. This is also a WTF that is not shared by most of Commerce contrib, which does it properly on hook_install().

So I support #6, and this is a patch that I've had in mind since before 1.0.
It's a good cleanup that shouldn't actually have any impact on things other than fixing this bug.

for example, when the Customer module is re-enabled after being disabled but not uninstalled that there are no new customer profile types requiring addressfields.

This can't happen, because a module that provides customer types will always depend on Customer, which means you can't have it enabled if Customer is not enabled. Do you have any other example that might prove problematic?

I tested the patch and it worked, I disabled commerce_shipping and commerce, re-enabled, everything still worked fine. That is currently impossible without the patch.
A hunch told me that the commerce_customer_modules_disabled() function should have a matching commerce_customer_modules_enabled() one that enables the previously disabled reference fields, but it seems that it works without that (would like an explanation why...).
Basically, I can confirm that when disabling only shipping, the disabled() function runs and the relevant customer_profile_reference field gets disabled. And then when enabling shipping again the field gets re-activated for no good reason :)

acrazyanimal’s picture

+1 interested

rszrama’s picture

Issue tags: +1.3 review

Tagging.

helior’s picture

Status: Needs review » Needs work

I can confirm that this patch makes it possible to disable and re-enable Commerce – exactly as advertised, however, only under the condition that modules are re-enabled in groups according to their correct order of dependencies:

Commerce

Price

Line Item
Product

Customer
Product Reference

Commerce UI
Order
Product Reference

Checkout
Order UI
Payment
Tax

Cart
Customer UI
Line Item UI
Payment Method Example
Payment UI
Product Pricing UI
Product UI
Tax UI

Was this the desired outcome, or was the goal to be able to re-enable all commerce modules at once without errors? I'll have to mark this as needs work until we can either get either some documentation on the process of re-enabling commerce modules, or possibly fix the various errors thrown when re-enabling all commerce modules at once.

bojanz’s picture

Did you test on a clean install with the patch applied? And what were the errors?
The patch basically moves all logic to install / uninstall, so enabling / disabling shouldn't actually do anything, which means there shouldn't be any errors ;)

helior’s picture

Yep, applied the patch and ran a clean install. Although the logic has been moved to install/uninstall, there is still an implementation of hook_modules_enabled() in the Payment module, which I believe is what invokes the error.

I attached the errors into a separate file. Each error is appended with a backtrace via Commerce Devel. Hope this helps :)

bojanz’s picture

Ah, okay, so we missed a spot. Shouldn't be hard to drive home then. I'll try and take a look soon if Andrei doesn't beat me to it.

vasike’s picture

first: i can confirm this issue
second: i can confirm both the #6 patch that let me re-enable the Commerce modules and the #13 error messages

Using the last dev Commerce

BrightBold’s picture

Same experience as #15 — #6 fixes the fatal error. You still get the errors in #15 but the site is back up. Pfew!

brephraim’s picture

Status: Needs work » Needs review
Issue tags: -1.3 review

#6: 1361562.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +1.3 review

The last submitted patch, 1361562.patch, failed testing.

nd987’s picture

Running into this problem also; mistakenly disabled a commerce module hoping to resolve a different issue :(

Has this patch been integrated with the latest dev snapshot?

kevinquillen’s picture

This appears to still be an issue. Disabling all commerce modules and uninstalling them causes 'field_some_field table' already exists errors.

rszrama’s picture

Status: Needs work » Needs review
FileSize
7.65 KB

There are a couple issues getting confused in these various installation / re-enabling threads:

  1. Some errors are caused by the modules attempting to create duplicate fields / instances, which the patch to move functionality to *_install() hooks instead of *_enable() would solve.
  2. We also have the issue of entity property info not being available when necessary, which is the source of helior's notices above.

Staratel has a patch to address #2 in #1509994: Missing entity property info results in notices / WSOD / fatal errors during installation, even though the issue really seems to have been created to address #1. In a separate issue opened to address #2, KarenS pointed out a quick fix to resolve #1 that doesn't involve moving functions around, #1430894-37: Recoverable fatal error: Argument 2 passed to entity_views_field_definition() must be an array. Some of these issues have been closed, and I think I just need to rename Staratel's issue to one that indicates it's solving the issue in the closed one KarenS commented in. Confused yet? : )

The attached patch is a less invasive fix than amateescu's patch, though it isn't a final solution. I'd like to get the test-bot to ok it and then maybe spawn a child task based on amateescu's patch to move the meat of these functions from the enabled process to installed. In the meantime, this patch would solve disabling / re-enabling for Commerce 1.6, though in a test I did end up losing a menu item provided by Views for some reason. C'est la vie.

Oh, and the approach of the patch is simply to always attempt to reactivate fields before creating them and clear the field cache more aggressively to ensure the necessary field info definitions are available and re-activated fields are accounted for.

rszrama’s picture

Status: Needs review » Fixed

Alrighty, I'm going to go ahead and commit this. Clean installations work as before in my local testing. The only minor API change is that commerce_field_activate() no longer automatically clears the field cache if it activates a disabled field. This is accommodated in our functions where necessary, but other modules calling this function will have to be updated. I expect they are few and far between.

Follow-up issue: #1962794: Move field creation code from enable hooks to install hooks

Commit: http://drupalcode.org/project/commerce.git/commitdiff/5e8829e

jpstrikesback’s picture

I arrived here after googling for "Recoverable fatal error: Argument 2 passed to entity_views_field_definition() must be an array, null given" which I encountered when enabling the 2.x branch of commerce recurring shortly after getting commerce 1.5. Pulling the latest dev fixed this it seems. Putting here for google.

Stack:

Recoverable fatal error: Argument 2 passed to entity_views_field_definition() must be an array, null given, called in .../sites/all/modules/entity/views/entity.views.inc on line 152 and defined in entity_views_field_definition() (line 175 of .../sites/all/modules/entity/views/entity.views.inc). Backtrace:
entity_views_field_definition('commerce_recurring_ref_product', NULL, Array) entity.views.inc:152
entity_views_table_definition('commerce_recurring') entity.views.inc:42
entity_views_data()
call_user_func_array('entity_views_data', Array) module.inc:857
module_invoke_all('views_data') cache.inc:76
_views_fetch_data_build() cache.inc:61
_views_fetch_data(NULL, 1, ) views.module:1247
views_fetch_data() entity.views.inc:320
entity_views_plugins() plugins.inc:414
views_discover_plugins() cache.inc:124
_views_fetch_plugin_data('display', 'default', ) views.module:1258
views_fetch_plugin_data('display', 'default') view.inc:2323
views_db_object->add_display('default', 'Master', 'default') view.inc:2419
views_db_object->new_display('default', 'Master', 'default') breeders_by_city.views_default.inc:24
breeders_by_city_views_default_views(Array) export.inc:680
_ctools_export_get_defaults('views_view', Array) export.inc:496
ctools_export_load_object('views_view', 'all') export.inc:148
ctools_export_crud_load_all('views_view', ) views.module:1434
views_get_all_views() views.module:1391
views_get_applicable_views('uses hook menu') views.module:400
views_menu_alter(Array, NULL, NULL) module.inc:1056
drupal_alter('menu', Array) menu.inc:2752
menu_router_build() menu.inc:2712
menu_rebuild() menu.inc:459
menu_get_item() menu.inc:1754
menu_get_custom_theme(1) menu.inc:1769
menu_set_custom_theme() common.inc:5130
_drupal_bootstrap_full() bootstrap.inc:2223
drupal_bootstrap(7) index.php:20

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

cleaver’s picture

I've been able to work around this problem by patching entity.views.inc

See: #2136919: Unsupported operand types in entity.views.inc when property info is missing.

cleaver’s picture