Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

civiimport - PEAR Exception handling - Mitigation of scenario where an import table has been deleted and the metadata is out of date #25633

Merged
merged 1 commit into from
Feb 22, 2023

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Feb 20, 2023

Overview

This fixes a situation where a fatal error on the main SearchKit screen which arises if the temp table for an import is deleted but the metadata cache for \Civi::cache('metadata')->get('api4.entities.info', []); still retains it.

This is most easily replicated by doing an import, deleting the temp table in the DB and then going to the search kit screen

Steps to reproduce:

  • Enable civiimport
  • Start an import. Upload a CSV. Press "Back" to submit a new upload.
  • In another tab, open /civicrm/admin/search#/list?tab=packaged.

Before

With Civi Import enabled....

image

After

image

Technical Details

Although a try-catch- was added to SearchKitit is by-passed by thePEAR_Exception. I could catch the PEAR_Exception` in SearchKit but SearchKit 'shouldn't have to' so handling in the ImportProvider / converting the exceptions.

try {
$getFields = civicrm_api4($entity['name'], 'getFields', [
'select' => ['name', 'title', 'label', 'description', 'type', 'options', 'input_type', 'input_attrs', 'data_type', 'serialize', 'entity', 'fk_entity', 'readonly', 'operators', 'suffixes', 'nullable'],
'where' => [['deprecated', '=', FALSE], ['name', 'NOT IN', ['api_key', 'hash']]],
'orderBy' => ['label'],
]);
}
catch (\CRM_Core_Exception $e) {
\Civi::log()->warning('Entity could not be loaded', ['entity' => $entity['name']]);
continue;
}

Thoughts about exceptions in #25600

Comments

I've figured out the scenario where this was happening & will separately address - but I think we should handle this regardless as I think there will always be some chance of it arising

Not strictly a regression but the handling that did not have enough cover to catch the PEAR_Exception is in 5.59

@civibot
Copy link

civibot bot commented Feb 20, 2023

(Standard links)

@civibot civibot bot added the 5.59 label Feb 20, 2023
@eileenmcnaughton eileenmcnaughton changed the title Mitigation of scenario where an import table has been deleted and the metadata is out of date PEAR Exception handling - Mitigation of scenario where an import table has been deleted and the metadata is out of date Feb 20, 2023
@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

really - still running/

test this please

@totten
Copy link
Member

totten commented Feb 22, 2023

Discussed on MM to figure out steps-to-reproduce. Updated description. Confirmed that the patch fixed. 👍

@totten totten changed the title PEAR Exception handling - Mitigation of scenario where an import table has been deleted and the metadata is out of date civiimport - PEAR Exception handling - Mitigation of scenario where an import table has been deleted and the metadata is out of date Feb 22, 2023
@totten totten merged commit 1cb8fbf into civicrm:5.59 Feb 22, 2023
@totten totten deleted the 559 branch February 22, 2023 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants