Improve error handling #71
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Naktibalda
reviewed
May 20, 2021
| 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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
The if-clause here should protect breaking any website when not running tests:
Also note that any errors encountered could already "break" a website, since a 500 response code is set when no headers are sent yet:
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 ( ) 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.
|
@Naktibalda Would this PR be acceptable for a new major version of c3? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improve error handling in a number of ways (see commit messages).
If not all parts of this PR are desirable in general, feel free to cherry pick.
Also fixes #41 and #37