fix: correctly set error code in the debug mode #198
Merged
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
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
WalkthroughThis update introduces a series of enhancements and bug fixes across the project. Key changes include improved error handling in Go code, the addition of a new test configuration for addressing a specific bug scenario, and updates to test cases to align with new configurations and address specific bugs. The changes aim to refine the project's robustness, particularly in error handling and testing, ensuring more accurate responses and behaviors under error conditions. Changes
Assessment against linked issues
Related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (8)
go.work.sumis excluded by:!**/*.sumtests/go.modis excluded by:!**/*.modtests/go.sumis excluded by:!**/*.sumtests/test-certs/localhost+2-client-key.pemis excluded by:!**/*.pemtests/test-certs/localhost+2-client.pemis excluded by:!**/*.pemtests/test-certs/localhost+2-key.pemis excluded by:!**/*.pemtests/test-certs/localhost+2.pemis excluded by:!**/*.pemtests/test-certs/rootCA.pemis excluded by:!**/*.pem
Files selected for processing (7)
- .gitignore (1 hunks)
- handler/handler.go (1 hunks)
- tests/configs/.rr-bug1843.yaml (1 hunks)
- tests/handler_test.go (1 hunks)
- tests/http_plugin3_test.go (2 hunks)
- tests/http_plugin4_test.go (1 hunks)
- tests/php_test_files/bug1843.php (1 hunks)
Additional comments: 8
tests/php_test_files/bug1843.php (1)
- 1-2: The PHP script immediately throws an exception, which is intended for testing the server's error handling before the PSR7 worker is initialized. This approach is effective for simulating early script failures.
.gitignore (1)
- 19-19: Adding
**/tests/test_certs/**to.gitignoreis a good practice to prevent test certificates from being tracked in version control, enhancing security.tests/configs/.rr-bug1843.yaml (1)
- 1-20: The configuration file is correctly set up to replicate the issue described in #1843, with appropriate server and HTTP settings for testing. This ensures the test environment is isolated and controlled.
tests/http_plugin4_test.go (1)
- 96-178: The test function
TestBug1843is well-structured, making an HTTP request to trigger the error condition and asserting the expected 500 status code and error message. This effectively validates the fix for the reported issue.handler/handler.go (1)
- 214-225: Refactoring
handleErrorto write an internal server error and set a special header if there are no free workers is a good improvement. Including the error message in the response when in debug mode is helpful for debugging.tests/http_plugin3_test.go (2)
- 505-505: Updating the version in the
config.Pluginstruct to "2023.3.1" aligns with the project's versioning scheme and ensures the test uses the correct configuration.- 611-617: Adding a delay before reading the directory contents and using
assert.NotContainsto check for file names is a more robust way to ensure temporary files are cleaned up after multipart form tests.tests/handler_test.go (1)
- 1463-1463: The server address in
TestHandler_Multipart_PATCHwas updated to:34432. Ensure this port is not used elsewhere in the tests or application to avoid conflicts.Verification successful
The search for the usage of port
34432within the codebase has returned only one occurrence, which is in the filetests/handler_test.goas specified in the review comment. This indicates that the port34432is not used elsewhere in the tests or application, thus avoiding potential conflicts.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the usage of port 34432 to ensure no conflicts. rg ":34432"Length of output: 117
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
go.work.sumis excluded by:!**/*.sumtests/go.modis excluded by:!**/*.modtests/go.sumis excluded by:!**/*.sum
Files selected for processing (2)
- .github/workflows/linux.yml (2 hunks)
- tests/http_plugin4_test.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/http_plugin4_test.go
Additional comments: 2
.github/workflows/linux.yml (2)
- 20-20: PHP version updated to 8.3 in the CI pipeline. This change ensures compatibility with the latest PHP version.
- 80-80: Added a step to create a
test-certsdirectory and populate it with SSL certificates. This is essential for HTTPS tests, ensuring the CI environment is correctly set up for security testing.
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.
Reason for This PR
closes: roadrunner-server/roadrunner#1843
Description of Changes
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.
PR Checklist
[Author TODO: Meet these criteria.][Reviewer TODO: Verify that these criteria are met. Request changes if not]git commit -s).CHANGELOG.md.Summary by CodeRabbit
.gitignoreto exclude specific test certificates.\RuntimeException.