WPTT / theme-sniffer

Theme Sniffer plugin using sniffs.
MIT License
269 stars 3 forks source link

Check in progress just spins #149

Closed sflwa closed 5 years ago

sflwa commented 5 years ago

I've tried running this on a couple of different sites (each on a different host / provider) and the check in progress just spins - I don't get the spinning mouse icon just the box - is this a PHP issue??

timelsass commented 5 years ago

I'm sorry to hear you're having issues @sflwa! it's not very easy to diagnose the issue without more information - if it is a PHP issue, output from your php error logs would help diagnose the problem. Theme Sniffer does also require PHP7 - so if those sites aren't running 7.0+ PHP then that could be a reason. There's a few reasons why it can get stuck with just the spinner going.

Probably the most common scenario right now is if PHPCS runs into issues with parsing - usually from encountering minfied CSS/JS or a timeout in PHP. If you're running PHP7+, then I would suggest trying the scan and checking the box to scan only PHP files and see if that works. Also, check your browser's console for any errors being output there.

sflwa commented 5 years ago

23-Mar-2019 23:42:27 UTC] PHP Warning: Invalid argument supplied for foreach() in /www/wp-includes/class-wp-list-util.php on line 150 [24-Mar-2019 00:26:57 UTC] PHP Warning: shell_exec() has been disabled for security reasons in /www/wp-content/plugins/theme-sniffer/vendor/squizlabs/php_codesniffer/src/Config.php on line 222 [24-Mar-2019 00:26:57 UTC] PHP Fatal error: Uncaught TypeError: Argument 1 passed to WordPress\Sniff::process() must be an instance of PHP_CodeSniffer_File, instance of PHP_CodeSniffer\Files\DummyFile given, called in /www/wp-content/plugins/theme-sniffer/vendor/squizlabs/php_codesniffer/src/Files/File.php on line 502 and defined in /www/wp-content/plugins/theme-sniffer/vendor/wp-coding-standards/wpcs/WordPress/Sniff.php:838 Stack trace:

0

/www/wp-content/plugins/theme-sniffer/vendor/squizlabs/php_codesniffer/src/Files/File.php(502): WordPress\Sniff->process(Object(PHP_CodeSniffer\Files\DummyFile), 0)

1

/www/wp-content/plugins/theme-sniffer/vendor/squizlabs/php_codesniffer/src/Runner.php(592): PHP_CodeSniffer\Files\File->process()

2

/www/wp-content/plugins/theme-sniffer/src/callback/class-run-sniffer-callback.php(595): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\DummyFile))

3

/www/wp-content/plugins/theme-sniffer/src/callback/class-run-sniffer-callback.php(461): Theme_Sniffer\Callback\Run_Sniffer_Callback->get_sniff_results(Arr in /www/wp-content/plugins/theme-sniffer/vendor/wp-coding-standards/wpcs/WordPress/Sniff.php on line 838 [image: Screen Shot 2019-03-23 at 8.34.58 PM.png]

On Fri, Mar 22, 2019 at 10:56 PM Tim Elsass notifications@github.com wrote:

I'm sorry to hear you're having issues @sflwa https://github.com/sflwa! it's not very easy to diagnose the issue without more information - if it is a PHP issue, output from your php error logs would help diagnose the problem. Theme Sniffer does also require PHP7 https://mailtrack.io/trace/link/bcab0feab79bef61f57fa2c1f078eb5e948b1ab6?url=https%3A%2F%2Fgithub.com%2FWPTRT%2Ftheme-sniffer%2Fblob%2Fdevelopment%2Freadme.txt%23L6&userId=3267200&signature=6c535e203d439c8a

  • so if those sites aren't running 7.0+ PHP then that could be a reason. There's a few reasons why it can get stuck with just the spinner going.

Probably the most common scenario right now is if PHPCS runs into issues with parsing - usually from encountering minfied CSS/JS or a timeout in PHP. If you're running PHP7+, then I would suggest trying the scan and checking the box to scan only PHP files and see if that works. Also, check your browser's console for any errors being output there.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://mailtrack.io/trace/link/4947015603ba8f6a337133e338f56c6a80b56f4c?url=https%3A%2F%2Fgithub.com%2FWPTRT%2Ftheme-sniffer%2Fissues%2F149%23issuecomment-475833397&userId=3267200&signature=a780439917079001, or mute the thread https://mailtrack.io/trace/link/74d447e4109f4160638b42bcd3a20605c8871299?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FANE8whkglz5VlMooBDzTPI_N8AaBtVK-ks5vZZfpgaJpZM4b9pX3&userId=3267200&signature=0a4592734b10b6a5 .

-- Philip Levine President South Florida Web Advisors, Inc. http://www.sflwa.net https://mailtrack.io/trace/link/531c738d226ad42baeb0702cc842ab7f213513a7?url=http%3A%2F%2Fwww.sflwa.net%2F&userId=3267200&signature=e3dc7d3b5123036f 7525 Northport Drive Boynton Beach, FL 33472 561-337-7806

timelsass commented 5 years ago

Ah thanks so much for posting back those logs! That makes things more clear :smiley: PHPCS is a command line tool, and triggering scripts like phpcs or wp-cli is usually handled via shell_exec calls. This is commonly disabled by hosts for security reasons. I think maybe we should add another check in activation to catch this to prevent use in those cases. Great catch @sflwa!

timelsass commented 5 years ago

Hey @sflwa - I have sent a PR that addresses this error. It doesn't fix the fact that the plugin won't work if the environment doesn't meet the plugin's requirements, but it will prevent activation of the plugin, so you won't run into this situation.

This is the built version with that change if you wanted to test: theme-sniffer.zip

sflwa commented 5 years ago

Now it at least generates the fatal error - personally I'd prefer an pretty error message but this is a quick fix :)

Thanks

On Sun, Mar 24, 2019 at 1:54 PM Tim Elsass notifications@github.com wrote:

Hey @sflwa https://github.com/sflwa - I have pushed up a change that addresses this error. It doesn't fix the fact that the plugin won't work if the environment doesn't meet the plugin's requirements, but it will prevent activation of the plugin, so you won't run into this situation.

This is the built version with that change if you wanted to test: theme-sniffer.zip https://mailtrack.io/trace/link/1292b4a65edf889d72ea031eaab0a2ca7510ef26?url=https%3A%2F%2Fgithub.com%2FWPTRT%2Ftheme-sniffer%2Ffiles%2F3000645%2Ftheme-sniffer.zip&userId=3267200&signature=2dbfdeb972e619b5

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://mailtrack.io/trace/link/e292abcdadf49918e770dde5d0cf98342823bdbe?url=https%3A%2F%2Fgithub.com%2FWPTRT%2Ftheme-sniffer%2Fissues%2F149%23issuecomment-475983116&userId=3267200&signature=3d6d8f75a55dce2b, or mute the thread https://mailtrack.io/trace/link/ac0aa7764a2c9a5e3908d0710d7ba58d08bce5ad?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FANE8wtb-8RvkP32yjtQ_YwDqGdUoiN58ks5vZ7vmgaJpZM4b9pX3&userId=3267200&signature=1d40df8d55592210 .

-- Philip Levine President South Florida Web Advisors, Inc. http://www.sflwa.net https://mailtrack.io/trace/link/61ad1b8feea0ccd35bb37c1d5e4fca021f0c5edc?url=http%3A%2F%2Fwww.sflwa.net%2F&userId=3267200&signature=03513de31ea7d2c0 7525 Northport Drive Boynton Beach, FL 33472 561-337-7806

timelsass commented 5 years ago

Haha, yeah, I'm not a fan of the notice displaying the fatal from a user side of things either ( it does this for incompatible PHP versions as well ). I'm glad that PR takes care of it in your environment!

I guess part of the reasoning for the notice might be this tool is geared towards technical users and developers, so some of them may find the trace useful for debugging/fixing. I think another reason might be since it's an exception on plugin activation that's also preventing extra code from having to be added just to create and handle making an admin notice since core handles exceptions. @dingo-d any thoughts or input?

dingo-d commented 5 years ago

The error on activation is something that is controlled by WordPress I think (not sure if the stack trace will show if the debug is disabled).

I've merged the PR and this will be solved in the next version.