WP_Stream_DB::store() and WP_Stream_Log::log() don't return true/false correctly
sc0ttkclark opened this issue · 7 comments
I'm finding it hard to determine if a Stream event has been logged from within a Stream connector, or any other place.
Can you pass true/false based on the $result in WP_Stream_DB::store() and ensure that true/false gets passed down to WP_Stream_Log::log() which it's called from?
return true;
after the do_action in WP_Stream_DB::store(), and return false;
below that statement because it would have not been successful ($result is empty or is_wp_error).
In WP_Stream_Log::log you could set $stored = WP_Stream::$db->store( array( $recordarr ) );
and return $stored;
at the end of the method.
You'll also want to update the log method @return
phpdoc, since it says void currently. The store() method says it returns boolean based on if it saved properly, so that is already set as expected.
@sc0ttkclark Thanks for pointing this out. Does this work for you?
Also, just as a side note: When the actual record is sent off to the WP Stream API for storage it's sent using a non-blocking request, so we can't actually verify if it saved or not. This would only be a problem for malformed records that the API would reject due to invalid data.
Gotcha, yeah this is concerning when planning on using Stream for logging more activity on a site for an item, with the possibility of data loss when the API becomes unavailable for whatever reason.
Maybe WP_Stream_DB::insert
could get $blocking
added and passed into WP_Stream_API::new_records
, which WP_Stream_DB::store
could set $blocking
through a filter, which gets passed $records
. The filter would get placed after the $records
themselves are filtered and confirmed not empty.
I can't reopen or I would, any way to address the $blocking enhancement so this could be used in a more precise way (when doing logging from crons which have no timeout / UI limitations)
@sc0ttkclark I think this thread and the blocking enhancement, while related, are really two separate issues.
I'll open a new issue then