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

Add logging for DB table management and creation #879

Closed
danieliser opened this issue Sep 18, 2020 · 5 comments
Closed

Add logging for DB table management and creation #879

danieliser opened this issue Sep 18, 2020 · 5 comments
Milestone

Comments

@danieliser
Copy link
Member

Describe the feature request

We should attempt to detect issues with table creation and log them for study.

Use case

Recent issues with the subscribers table.

@fpcorso fpcorso modified the milestones: v1.13, v1.14 Sep 30, 2020
@fpcorso
Copy link
Contributor

fpcorso commented Nov 5, 2020

@danieliser The only table we have is the subscribers table. The table gets created using dbDelta which doesn't really give us much insights to log: https://github.com/PopupMaker/Popup-Maker/blob/master/classes/DB/Subscribers.php#L115

dbDelta does return a string of the "results" but, even according to the docs, there are times that it returns what was expected to happen as opposed to what actually happened. E.g. it might return "Created table pum_subscribers" even though it wasn't created: https://developer.wordpress.org/reference/functions/dbdelta/

So, we can catch that and log it to our error log but we may also take it a step farther by immediately checking if the table exists and then logging if it doesn't. Unfortunately, that still doesn't actually give us any insights as to cause and one could easily check to see if the table exists so not sure if that would actually help us.

Though, dbDelta does a $wpdb->query as the final step so, I wonder if we could use the wpdb errors in some way...

@fpcorso
Copy link
Contributor

fpcorso commented Nov 5, 2020

@danieliser This is what logging the results of dbDelta looks like:
image

At least if dbDelta returns some error info, we will now catch it.

I'll play with the other ideas too to see what I can come up with.

fpcorso added a commit that referenced this issue Nov 5, 2020
@fpcorso
Copy link
Contributor

fpcorso commented Nov 5, 2020

@danieliser Added a check that immediately follows the creation to determine if the table exists, which worked when testing. But, I can't get wpdb to add the last error so there must be something flushing that during dbDelta or right after, still exploring...

Also, as shown, the dbDelta said the table was created even though many errors appeared on screen and the table ultimately did not get created (due to a purposely misspelling of keywords).

image

@fpcorso
Copy link
Contributor

fpcorso commented Nov 5, 2020

@danieliser Ah! My own exists check was causing the last_error to be set to a blank string 🤦‍♂️. So, saving the last error prior to my check is the trick:

image

I think that takes care of this issue and gets us the info I think we were after.

fpcorso added a commit that referenced this issue Nov 5, 2020
If the table is not found, log the last error from wpdb to our error
log.

Issue #879
@fpcorso fpcorso closed this as completed Nov 5, 2020
@danieliser
Copy link
Member Author

danieliser commented Nov 9, 2020

@fpsorso Oh that looks really nice now, should help us track down these edge cases better for sure.

@fpcorso fpcorso mentioned this issue Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants