wp-rocket icon indicating copy to clipboard operation
wp-rocket copied to clipboard

Better handling RUCSS errors

Open piotrbak opened this issue 2 years ago • 2 comments

Before submitting an issue please check that you’ve completed the following steps:

  • Made sure you’re on the latest version
  • Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug

  1. If job wasn't successful, we don't store any data why it failed
  2. When SaaS receives timeout, we're increasing the retries column up to 3 and then change the status to failed. For each retry we're fetching the same Job Details

Expected behavior

  1. If job wasn't successful, we should save the last status code and the last error message to the database, erasing the previous messages. We'll need a new column to achieve that.
  2. We should create a new job in RUCSS for each retry (3 retries) instead of fetching the same job details.

Additional context Additional context: https://wp-media.slack.com/archives/C029G1B5HD2/p1662458337836339?thread_ts=1662099835.213589&cid=C029G1B5HD2

@engahmeds3ed @mostafa-hisham since you were discussing this problem, I believe that you should do grooming there.

Backlog Grooming (for WP Media dev team use only)

  • [ ] Reproduce the problem
  • [ ] Identify the root cause
  • [ ] Scope a solution
  • [ ] Estimate the effort

piotrbak avatar Sep 07 '22 12:09 piotrbak

Scope a solution

  • Respond with a different error code (504) on timeout on the SAAS side
  • Update RUCSS controller to create a new JOB on 504 errors
  • create a new function to create new jobs and use it in tree shake and in timeout error from https://github.com/wp-media/wp-rocket/blob/cdf8f7bd07a2566b800d13f0981bf5babfd5b63f/inc/Engine/Optimization/RUCSS/Controller/UsedCSS.php#L229-L262
  • in check_job_status check if the status is 504 first and then create a new Job https://github.com/wp-media/wp-rocket/blob/cdf8f7bd07a2566b800d13f0981bf5babfd5b63f/inc/Engine/Optimization/RUCSS/Controller/UsedCSS.php#L600-L620

To store the error message

  • update inc/Engine/Optimization/RUCSS/Database/Row/UsedCSS.php inc/Engine/Optimization/RUCSS/Database/Schemas/UsedCSS.php inc/Engine/Optimization/RUCSS/Database/Tables/UsedCSS.php to add a new DB longtext column error_message

  • update make_status_failed inc/Engine/Optimization/RUCSS/Database/Queries/UsedCSS.php to store the error message from the response

  • Finally, update tests

Estimate effort:

Effort M

mostafa-hisham avatar Sep 08 '22 09:09 mostafa-hisham

@engahmeds3ed @mostafa-hisham I just updated the description of the issue.

piotrbak avatar Sep 13 '22 15:09 piotrbak