berlindb / core

All of the required core code
MIT License
252 stars 27 forks source link

Add a hook to is_success method #56

Open alexstandiford opened 4 years ago

alexstandiford commented 4 years ago

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 commented 4 years ago

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...

🤔

alexstandiford commented 4 years ago

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.