Skip to content

Conversation

@ahmad-cit22
Copy link
Contributor

@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
Contributor 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
@taylorotwell taylorotwell merged commit ba260b7 into laravel:11.x Nov 19, 2024
30 of 31 checks passed
@ahmad-cit22 ahmad-cit22 deleted the request-port-extraction-issue branch November 20, 2024 07:38
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