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 a hook to is_success method #56

Open
alexstandiford opened this issue Jun 30, 2020 · 2 comments
Open

Add a hook to is_success method #56

alexstandiford opened this issue Jun 30, 2020 · 2 comments
Assignees
Labels
type-request New feature or request
Milestone

Comments

@alexstandiford
Copy link
Collaborator

It would be nice if, after is_success() ran, if an action would fire. This would allow plugins to capture database events, and potentially log when something goes wrong. It would also make it possible to take a corrective action, if possible.

@JJJ
Copy link
Collaborator

JJJ commented Jun 30, 2020

That's a neat idea. I can see the value in it.

By itself, is_success() is not designed to be particularly useful. It's mostly a wrapper for the empty() and is_wp_error() checks together, and to be an abstraction for the WordPress function call – if the logic needs to grow, it has a place to do so.

As is, I think it lacks sufficient context to be particularly helpful. Currently, I imagine you'd need to use something like wp_debug_backtrace_summary() or current_action() to narrow down the scope of the call, which is pretty unappealing.

On the opposite end of the spectrum, Berlin may benefit from its own Error type class and handlers (ala WP_Error); something with IDs and descriptions and... usefulness – but maybe that's a problem more suited to the environment to solve than for us to maintain, too.

Berlin could go as far as installing its own database table(s) for storing its own operational logs. Logs could be a separate repository in this organization that is optionally installed and "just works" while also serving as a decent example of how to use it, since it would need its own Table, Row, Query, etc...

🤔

@JJJ JJJ added this to the Ongoing milestone Jun 30, 2020
@JJJ JJJ added the type-request New feature or request label Jun 30, 2020
@alexstandiford
Copy link
Collaborator Author

alexstandiford commented Jun 30, 2020

Well, the lack of context isn't entirely true. If is_success failed, often times there is an error in $wpdb->last_error that can be fetched at that moment to get an understanding on why it failed.

This request came originally from the realization that I have had several instances where my create table syntax messed up because of a trailing comma, or something, and I always have a fair amount of debugging that happens before I realize that was the cause. This may be a non-issue if we make the create table syntax end up being auto-generated from the schema, but in the meantime the only way I am able to capture this is through is_success.

That being said, perhaps I'm thinking too small. Maybe the problem in this circumstance has less to-do with needing a louder is_success method, and instead needing BerlinDB, as a whole, more vocal about what it did, and what went wrong, which I think comes back to your suggestion that this problem would be better served by implementing some sort-of extendable error logging class.

Side note, I think calling this error logging is a misnomer because we don't always want to log errors. Sometimes we just want to log when something happens. From here on, I'll refer to this as an event, or an event type

Underpin's logger utility makes error logging extendable by implementing a swappable Writer class for event types. Fundamentally, the default writer writes to a file, but the class that is used by an event logger is filterable, and can be swapped out with different Writer classes.

There's some documentation on that here: https://github.com/Underpin-WP/logger-loader#writers
You can see the abstraction for an event type here: https://github.com/Underpin-WP/logger-loader/tree/master/lib/abstracts
Here's a basic Error event type: https://github.com/Underpin-WP/logger-loader/blob/afe8c1151fa93dccad08111d00d8c986dc3e6f1d/lib/loaders/Logger.php#L67-L76

Underpin also uses these events with a built-in debug tool that turns on when WP_DEBUG is enabled. Basically this adds an interface that shows all of the events that were logged during that particular request.

image

I'm not convinced that Berlin, specifically, should have something like this, however I do think it should be equipped to easily integrate with systems that use BerlinDB and have their own logging utilities. It would be extremely useful to ensure whatever is created for BerlinDB can be compatible with debugging utilities that the rest of the plugin uses, regardless of what that system looks like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants