Skip to content
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.

Sign up for GitHub

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

Improve error handling #71

Open
wants to merge 3 commits into
base: 2.0
Choose a base branch
from
Open

Improve error handling #71

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f7811b9
Fix notice when handling c3 error before C3_CODECOVERAGE_MEDIATE_STOR…
marcovtwout May 19, 2021
ca3b3de
Attempt to write error log file even if it doesn't exist yet
marcovtwout May 19, 2021
02e0cb7
Echo c3 error to screen and stop execution.
marcovtwout May 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions c3.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,27 @@
if (!function_exists('__c3_error')) {
function __c3_error($message)
{
$errorLogFile = defined('C3_CODECOVERAGE_ERROR_LOG_FILE') ?
C3_CODECOVERAGE_ERROR_LOG_FILE :
C3_CODECOVERAGE_MEDIATE_STORAGE . DIRECTORY_SEPARATOR . 'error.txt';
if (is_writable($errorLogFile)) {
file_put_contents($errorLogFile, $message);
if (defined('C3_CODECOVERAGE_ERROR_LOG_FILE')) {
$errorLogFile = C3_CODECOVERAGE_ERROR_LOG_FILE;
} elseif (defined('C3_CODECOVERAGE_MEDIATE_STORAGE')) {
$errorLogFile = C3_CODECOVERAGE_MEDIATE_STORAGE . DIRECTORY_SEPARATOR . 'error.txt';
} else {
$message = "Could not write error to log file ($errorLogFile), original message: $message";
$errorLogFile = null;
}

if (isset($errorLogFile)) {
if (@file_put_contents($errorLogFile, $message) === false) {
$message = "Could not write error to log file ($errorLogFile), original message: $message";
}
}

if (!headers_sent()) {
header('X-Codeception-CodeCoverage-Error: ' . str_replace("\n", ' ', $message), true, 500);
}
setcookie('CODECEPTION_CODECOVERAGE_ERROR', $message);

echo 'CodeCeption C3 Error: ' . $message;
exit(1);
Copy link
Member

@Naktibalda Naktibalda May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block is actually a major change in behaviour.
Before, c3 set a cookie and website continued to work as usual, now it fails loud and clear and the website doesn't work. It isn't a problem for tests, but, if c3.php is included to live website, it would break website for real users.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if-clause here should protect breaking any website when not running tests:

c3/c3.php

Lines 34 to 36 in f7811b9

if (!array_key_exists('HTTP_X_CODECEPTION_CODECOVERAGE', $_SERVER)) {
return;
}

Also note that any errors encountered could already "break" a website, since a 500 response code is set when no headers are sent yet:

c3/c3.php

Line 54 in f7811b9

header('X-Codeception-CodeCoverage-Error: ' . str_replace("\n", ' ', $message), true, 500);

I know this part changes behavior when running tests and is perhaps better suited for a new major version. There's actually two points to this change:

  • Echo error message. Possibly makes it easier to debug for first-time users. Showing details could be made optional.
  • Stop c3 collection when encountering error. Most calls to __c3_error() look like they should stop execution of c3, but do not. For example, when Codeception is not loaded an error is thrown (

    c3/c3.php

    Lines 72 to 74 in f7811b9

    } else {
    __c3_error('Codeception is not loaded. Please check that either PHAR or Composer package can be used');
    }
    ) but execution continues. This could lead to more errors or unexpected behavior. Another way to solve this is to make those calls throw proper exceptions and catch them to log error and continue normal application execution.

}
}

Expand Down