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.

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

[11.x] Fix: Improve Request Port Extraction Handling in ServeCommand.php to Prevent Artisan Command Failures #53538

Open
wants to merge 6 commits into
base: 11.x
Choose a base branch
from

Conversation

ahmad-cit22
Copy link

@ahmad-cit22 ahmad-cit22 commented Nov 16, 2024

[Updated] This PR fixes an issue (#53534) in the getRequestPortFromLine method in ServeCommand.php where failing to extract the request port from the log line resulted in an Undefined array key 1 error when using LOG_CHANNEL=stderr. This error would break the server when generating a URL with missing parameters, causing Artisan commands to fail.

Changes:

  • Updated Regex: The regular expression was updated to match log lines with an optional datetime prefix, ensuring proper extraction of the request port, even when a timestamp is present.
  • Improved error handling: If the port number cannot be extracted, the method will now throw a clear exception with the problematic log line, making debugging easier.

How This Fix Solves the Issue:

This fix resolves the issue where the server breaks because the getRequestPortFromLine method fails to parse log lines with a datetime prefix. By updating the regex, we ensure that both types of log lines (with and without datetime) are handled correctly. Additionally, the exception handling ensures that if parsing fails, it is immediately clear which log line caused the failure, facilitating quicker debugging for devs.


This way, this PR enhances the stability of Laravel's artisan serve command by addressing the log parsing issue in a a manner which can ensure smoother development workflows. Thank you.

@ahmad-cit22 ahmad-cit22 changed the title Fix: Improve Request Port Extraction Handling in ServeCommand.php to Prevent Artisan Command Failures [11.x] Fix: Improve Request Port Extraction Handling in ServeCommand.php to Prevent Artisan Command Failures Nov 16, 2024
@taylorotwell
Copy link
Member

Can we extract this function into a static function that we then put a lot of tests around?

@taylorotwell taylorotwell marked this pull request as draft November 18, 2024 21:55
@ahmad-cit22
Copy link
Author

ahmad-cit22 commented Nov 19, 2024

Can we extract this function into a static function that we then put a lot of tests around?

Thanks a lot.

Based on your suggestion, I moved the getRequestPortFromLine logic into a new static method in the LogParser utility class which ensures better testability and separation of concerns. There, I introduced a more specific InvalidArgumentException when the port cannot be parsed.
And then wrote unit tests for the function considering various types of log lines as well as possible edge cases for invalid lines.

Hope this will solve the issue in a better approach.

@ahmad-cit22 ahmad-cit22 marked this pull request as ready for review November 19, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UrlGenerationException sends a log line to stderr that breaks artisan serve
3 participants