-
Notifications
You must be signed in to change notification settings - Fork 39
Log to file #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Log to file #84
Conversation
- use include pattern instead of rendering in class
- download zip file including logs and information about setup
- added utility classes
…task/logging-troubleshooting
src/class-tiny-compress-client.php
Outdated
|
|
||
| // Set API request timeout to prevent indefinite hanging | ||
| $options[ CURLOPT_TIMEOUT ] = self::API_TIMEOUT; | ||
| $options[ CURLOPT_CONNECTTIMEOUT ] = self::API_TIMEOUT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the connecttimeout should be shorter (like 10 seconds)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently set to 8. Let me know if this is okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds logging and diagnostics capabilities to help troubleshoot customer issues. It introduces a file-based logger with automatic rotation at 2MB, a diagnostics export feature that bundles logs and system information into a downloadable zip file, and sets API request timeouts (120s total, 8s connect) to prevent indefinite hanging.
Changes:
- Introduced
Tiny_Loggersingleton for file-based logging with 2MB rotation - Added
Tiny_Diagnosticsclass to collect system info and generate downloadable diagnostics zip files - Integrated logging calls throughout the compression workflow
- Added settings UI for enabling/disabling logging and downloading diagnostics
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tiny-compress-images.php | Added requires for new logger and diagnostics classes |
| test/unit/TinySettingsAjaxTest.php | Refactored test to use new assertHook helper method |
| test/unit/TinySettingsAdminTest.php | Added test for new logging_enabled setting registration |
| test/unit/TinyLoggerTest.php | Added comprehensive tests for logger singleton, file creation, rotation, and enable/disable behavior |
| test/unit/TinyDiagnosticsTest.php | Added tests for diagnostics action hook and info collection |
| test/helpers/wordpress.php | Added assertHook helper, WordPressMocks class with mock WordPress functions, and formatting updates |
| src/views/settings.php | Included new diagnostics settings partial |
| src/views/settings-diagnostics.php | New view for logging toggle and diagnostics download button |
| src/js/admin.js | Added AJAX handler for diagnostics download |
| src/css/admin.css | Added utility classes for diagnostics UI |
| src/class-tiny-settings.php | Instantiated diagnostics and registered logging_enabled setting |
| src/class-tiny-plugin.php | Initialized logger and added debug logging throughout compression flow |
| src/class-tiny-logger.php | New singleton logger with file rotation and WordPress option integration |
| src/class-tiny-image.php | Added detailed debug and error logging for compression operations |
| src/class-tiny-diagnostics.php | New class for collecting and exporting diagnostic information |
| src/class-tiny-compress-fopen.php | Added debug logging for fopen compression responses |
| src/class-tiny-compress-client.php | Added timeout constants, error logging, and applied timeout settings to client |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| /** | ||
| * Constructor. | ||
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing closing line in docblock. The docblock comment starting at line 54 is not properly closed before the function definition.
| * |
| * https://developer.wordpress.org/reference/classes/wpdb/db_version/ | ||
| * | ||
| * @return string|null database version | ||
| */ |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect docblock reference. The db_version function is documented as a wpdb class method, but this is a standalone mock function. The documentation link and description should reflect that this is a mock implementation.
| * https://developer.wordpress.org/reference/classes/wpdb/db_version/ | ||
| * | ||
| * @return string|null database version |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect docblock - wp_get_theme is documented with the db_version reference and return type. The documentation should reference the correct WordPress function and return a WP_Theme object description.
| * https://developer.wordpress.org/reference/classes/wpdb/db_version/ | |
| * | |
| * @return string|null database version | |
| * Mocked function for https://developer.wordpress.org/reference/functions/wp_get_theme/ | |
| * | |
| * @return \WP_Theme Mock WP_Theme-like object |
| * https://developer.wordpress.org/reference/classes/wpdb/db_version/ | ||
| * | ||
| * @return string|null database version |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect docblock - get_home_url is documented with the db_version reference and return type. Should reference the correct WordPress function and return type (string URL).
| * https://developer.wordpress.org/reference/classes/wpdb/db_version/ | |
| * | |
| * @return string|null database version | |
| * https://developer.wordpress.org/reference/functions/get_home_url/ | |
| * | |
| * @return string Home URL |
| * https://developer.wordpress.org/reference/classes/wpdb/db_version/ | ||
| * | ||
| * @return string|null database version |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect docblock - get_locale is documented with the db_version reference and return type. Should reference the correct WordPress function and return type (string locale code).
| * https://developer.wordpress.org/reference/classes/wpdb/db_version/ | |
| * | |
| * @return string|null database version | |
| * https://developer.wordpress.org/reference/functions/get_locale/ | |
| * | |
| * @return string Locale code |
| /** | ||
| * https://developer.wordpress.org/reference/functions/wp_timezone_string/ | ||
| * | ||
| * @return string|null database version |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect return type description. The function returns a timezone string, not a database version.
| * @return string|null database version | |
| * @return string|null Timezone string |
| return 'timezone'; | ||
| } | ||
| /** | ||
| * https://developer.wordpress.org/reference/functions/wp_timezone_string/ |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect docblock reference. The update_option function is documented with the wp_timezone_string reference. Should reference https://developer.wordpress.org/reference/functions/update_option/
| * https://developer.wordpress.org/reference/functions/wp_timezone_string/ | |
| * https://developer.wordpress.org/reference/functions/update_option/ |
| * @since 3.6.8 | ||
| * @var int | ||
| */ | ||
| const API_TIMEOUT = 120; |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing docblock for CONNECT_TIMEOUT constant. The API_TIMEOUT constant has documentation but CONNECT_TIMEOUT does not.
| const API_TIMEOUT = 120; | |
| const API_TIMEOUT = 120; | |
| /** | |
| * Connection timeout in seconds. | |
| * | |
| * @since 3.6.8 | |
| * @var int | |
| */ |
| */ | ||
| public function get_locale() | ||
| { | ||
| return 'gb_GB'; |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected locale code from 'gb_GB' to 'en_GB'. The correct WordPress locale code for Great Britain is 'en_GB', not 'gb_GB'.
| return 'gb_GB'; | |
| return 'en_GB'; |
| 'Whenever you run into issues, we can help faster if we can see what is happening. | ||
| Please enable logging for a short period, reproduce the problem, then send us a message at support@tinify.com with the diagnostics file attached.', |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long multi-line string has inconsistent indentation. The second line has leading whitespace that will appear in the output. Consider using a single line or properly formatting the multi-line string to avoid unintended whitespace.
| 'Whenever you run into issues, we can help faster if we can see what is happening. | |
| Please enable logging for a short period, reproduce the problem, then send us a message at support@tinify.com with the diagnostics file attached.', | |
| 'Whenever you run into issues, we can help faster if we can see what is happening. Please enable logging for a short period, reproduce the problem, then send us a message at support@tinify.com with the diagnostics file attached.', |
We had two customers reporting two different issues which we could not figure out. Often times this is due to setup configurations but it is hard to figure out.
Having some diagnostics and logs should make it easier to troubleshoot these issues.
This PR will also:
Good to know
Screenshots

Customers can turn on logging and download a diagnostics zip file from within settings