nystudio107 / craft-retour

Retour allows you to intelligently redirect legacy URLs, so that you don't lose SEO value when rebuilding & restructuring a website
https://nystudio107.com/plugins/retour
Other
38 stars 26 forks source link

Integrity constraint violation errors being logged #305

Closed peteeveleigh closed 3 months ago

peteeveleigh commented 3 months ago

Describe the bug

On a couple of sites we are seeing the occasional Integrity constraint violation error popping up but are unable to determine why these are happening.

An example error message from the logs.

`SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '/products/favicon.ico' for key 'craft_retour_stats_redirectSrcUrl_unq_idx'
The SQL being executed was: INSERT INTO `craft_retour_stats` (`siteId`, `redirectSrcUrl`, `referrerUrl`, `remoteIp`, `userAgent`, `exceptionMessage`, `exceptionFilePath`, `exceptionFileLine`, `hitCount`, `hitLastTime`, `handledByRetour`, `dateCreated`, `dateUpdated`, `uid`) VALUES (1, '/products/favicon.ico', '', '104.243.223.12', 'Client/42205 CFNetwork/1496.0.7 Darwin/23.5.0', 'Page not found.', '/home/forge/www.redacted.com/vendor/craftcms/cms/src/web/Request.php', 1353, 1, '2024-06-16 00:39:21', 0, '2024-06-16 00:39:21', '2024-06-16 00:39:21', '114d54e4-033c-4662-a9d7-a4ca67b4d2f5')`

What is peculiar is that if I search the craft_retour_stats table then there is a row for '/products/favicon.ico' but no such redirect exists within the craft_retour_static_redirects table.

I can see that the craft_retour_stats table records any 404s regardless of whether or not a redirect exists. So I have tested hitting a non-existent URL and see that appear in the craft_retour_stats table. Then tried hitting the same URL again, but no constraint violation occurs (nothing else appears in the table).

The error pops up a few times a month and I'm unable to reproduce it on-demand so I understand it would be difficult to diagnose, but I wondered if you had any ideas as to why this might happen.

To reproduce

Steps to reproduce the behaviour:

  1. Unable to reproduce the error on-demand

Expected behaviour

Ideally, no integrity constraint error

Screenshots

N/A

Versions

khalwat commented 3 months ago

So I applied a "fix" via the above commits based on code inspection. I'm pretty sure this will fix it, though it's tough to say for sure because I can't reproduce exactly what's going on in your setup here locally.

However, the fix makes sense.

What is happening is Retour creates a new RetourStats model with redirectSrcUrl set to the incoming $url here: https://github.com/nystudio107/craft-retour/blob/develop-v5/src/services/Statistics.php#L192

Then it does a query where it looks to see if an existing statistics exists, otherwise it will create a new one. But then it cleans up the text for the redirectSrcUrl query parameter here: https://github.com/nystudio107/craft-retour/blob/develop-v5/src/services/Statistics.php#L198

So if the cleaned up redirectSrcUrl is different than the non-cleaned up incoming $url and that statistic doesn't exist, it will then try to save a new statistic but with redirectSrcUrl set to the non-cleaned up text... which might already exist, thus causing the exception.

Craft CMS 3:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-retour": "dev-develop as 3.2.16”,

Then do a composer clear-cache && composer update

…..

Craft CMS 4:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-retour": "dev-develop-v4 as 4.1.17”,

Then do a composer clear-cache && composer update

…..

Craft CMS 5:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-retour": "dev-develop-v5 as 5.0.1”,

Then do a composer clear-cache && composer update

peteeveleigh commented 3 months ago

Fantastic, thanks! I'll get this installed in the morning and monitor to see if we get it happening again. 👍

khalwat commented 3 months ago

I tagged a release, so you should now just need to update to the latest release.