wp-media / imagify-plugin

Speed up your website with lighter images without losing quality.
https://imagify.io
69 stars 24 forks source link

Closes #674 Asynchronous bulk optimization #693

Closed Tabrisrp closed 1 year ago

Tabrisrp commented 1 year ago

Make the bulk optimization asynchronous

Closes #674

joejoe04 commented 1 year ago

Doing some testing and noticing async Bulk Optimization process is not working for me when using v2.1 on my test site (It's not working for me whether I stay on the Bulk Opti page or not): https://i.joedisalvo.com/

Almost every time I try, no images will optimize at all, though, on a couple of attempts (out of very many), it randomly worked to optimize images.

However, if I go to the Media Library and try to optimize using the "Optimize" button, it works fine every time, and images are successfully optimized. The issue seems to be only related to bulk optimization.

Not getting any errors in debug.log.

Also, I see this error in the console sometimes (though not always):

upload.php:1 Unchecked runtime.lastError: A listener indicated an asynchronous response by returning true, but the message channel closed before a response was received

To reproduce the console errors, they happen sometimes (not always) when I try to run the Bulk Optimizer using version 2.0 and 2.1. The console errors don't start appearing until I allow it to run for some time.

Just mentioning these in case they happen in others' testing as well. For me, they almost seem random, happen sometimes when bulk optimization is not working, but also sometimes happen when it is working too, so it seems like it might not be related to the issue I'm having with Bulk Optimization.

If I go back to version 2.0, bulk optimization works fine.

Not getting consistent results, but if I had to summarize what seems to be happening:

piotrbak commented 1 year ago

@Tabrisrp In my case the image optimisation process got stuck at 34% and I'm receiving the same values in the admin-ajax call.

Do we have any access to the WP Background Processing queue? I think that's the next step to investigate as cron is working correctly.

Tabrisrp commented 1 year ago

@piotrbak In the options table, during optimization, you should see entries like the following:

Those identify the background processes and the current status for each image

piotrbak commented 1 year ago

@Tabrisrp The process started working again, most likely after clicking Imagify em all once again. On the other hand, I believe that this button should not be available after the refresh if the process is still running 🤔

piotrbak commented 1 year ago

@Tabrisrp I received also that kind of message suddenly: image

After some time it was closed and success message was displayed: image

Now API is up again and the process is being continued.

piotrbak commented 1 year ago

@Tabrisrp It got stuck on my website again, now on 70%. The current state is:

  1. The process is stuck on 70%
  2. The button Imagify em all is not disabled 🆗
  3. There are no entries in the wp_options table that are matching the transient_imagify_(.*)locked there are also no imagify_optimize_media_batch_ entries in the wp_options

I believe that points 1 and 3 will be fixed as soon as we click the button from point 2nd. Do you want to take a look at this problem? I confirmed on a different site that those entries can disappear.

Another problem: 🆗 On a smaller site when I was able to reach 100%, instead of seeing success banner on Imagify page I can see this error: image (this is the first admin-ajax request that reached 100%)

piotrbak commented 1 year ago

Two problems are fixed, I'll see if the process being stuck is reproducible.

Moreover, when deactivating/deleting the plugin we should clean up all the data. When I removed the plugin and reinstalled it the button was still in disabled state.

Tabrisrp commented 1 year ago

Good catch I added a deletion of the transients on deactivation

piotrbak commented 1 year ago

@Tabrisrp

  1. There's one thing more related to the notice/banner. Can we make sure that each of them can be seen only once?

Right now if you see the notice and click to check the stats, the banner won't be visible. The same with the opposite. It would be great to make sure that after clicking on the Check stats on the WP Admin notice we'll see the stats in banner + CTA with leaving a review.

(It's not clearly stated in the epic card)

  1. I'm still struggling with process being stuck after some time. All the data from wp_options related to optimization process disappears (imagify_optimize_mediabatch). I can see some timeout values in the database, can it be related? image image
joejoe04 commented 1 year ago

Just to update my experience with the most recent version, I ran an optimization in the background last night of about 70 images, and when I checked this morning, it seemed to have worked well and without issue for both Media Library and custom folder images.

I was wondering about how the process could be stopped if it ever needed to be (for example, if every attempt at optimization was failing due to some error). I guess deactivating the plugin would be the only way?

Would it be useful to have a button or some option to stop the process if need be?

Tabrisrp commented 1 year ago

There is some things in place to display errors and stop the process if we get unexpected results from the API, but maybe we could have something to stop the process, though I think with the current release time we want, it would be an enhancement for later.

piotrbak commented 1 year ago

@Tabrisrp It's stuck with this commit too: https://github.com/wp-media/imagify-plugin/pull/693/commits/a5c3fa9faf09c184f7116586b0e3ff8b4172b2aa

Again, the nothing in the wp_options related to the process :/

piotrbak commented 1 year ago

On the different page it's stuck too: https://valentina.rocketlabsqa.ovh/

It's like it could not optimize two images 1 and 2.

I can see that:

  1. Actions in the AS are created
  2. When processed, in wp_options we have two entries related to them
  3. Then entries disappear but nothing changes in the UI
  4. When looking at them, they're not optimised image
  5. They can be optimised using https://app.imagify.io/ image
piotrbak commented 1 year ago

@Tabrisrp Do you think we can somehow separate those two cases?

  1. When the image is broken, would it be possible to remove it from the unoptimized data in request? Do you have any other idea? It'd be also important to guard against those log entries when we find the broken image:
    [09-Nov-2022 21:33:06 UTC] PHP Notice:  Undefined index: height in /var/www/rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify-plugin/classes/Optimization/File.php on line 828
    [09-Nov-2022 21:33:06 UTC] PHP Notice:  Undefined index: width in /var/www/rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify-plugin/classes/Optimization/File.php on line 827
  2. We still need to investigate why sometimes the process stucks, even when images are totally fine. It's happening now here, for example: https://valentina.rocketlabsqa.ovh. I might do some steps, but not sure how to move forward.
joejoe04 commented 1 year ago

Was doing more testing with most recent version () and I was getting these errors, no images were optimizing on my first 3 attempts at bulk optimization, and the process just stopped completely after some time passed:

image

On the 4th attempt, there were no errors and images started optimizing.

GeekPress commented 1 year ago

I just tried the Bulk Optimization and I got a couple of bugs. I'm going to list them one by one.

  1. Two problems explained in this video: https://share.getcloudapp.com/04uybOXl

  1. Once the Bulk is running, the row of the type of media isn't updated. We still have access to the checkbox. It must be replaced by the previous loader.

Current UI: image

Expected UI: image


  1. Do not display 0 0 0 when no images have been yet optimized:

Current UI: image

Expected UI:


  1. For error count, display it only when we have at least 1 error. We don't need to display 0 if we have no error.

Current UI: image

Expected UI: image