-
Notifications
You must be signed in to change notification settings - Fork 135
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
Additional debug message #29
base: master
Are you sure you want to change the base?
Conversation
I spent a lot of time figuring out why my script cannot be executed (502 Gateway error), it turns out that my Bash script is missing Shebang line. It would be great if a debug message can be printed out so that I do not need to hack the code to find out why. |
@@ -578,6 +578,7 @@ static void handle_fcgi_request(void) | |||
} | |||
|
|||
execl(filename, filename, (void *)NULL); | |||
fprintf(stderr, "Error: %s\n", strerror(errno)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about being more verbose than just "Error"? "Failed to execute script" or something like:
fprintf(stderr, "Failed to execute %s: %s\n", filename, strerror(errno));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not against removing the "Error", but cgi_error() function will also output "Cannot execute script ....", then it is kind of redundant to print "Failed to execute", but printing only the strerror(errno) without "Error" makes it a little confusing. Any idea on this?
@Lekensteyn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to add a parameter to cgi_error
that can be used to append additional information that should only be logged to the error log (instead of stdout). This can then be NULL for everything except the internal server error. What do you think of that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to keep consistent to the current definition of cgi_error
, because adding additional argument will make all the cgi_error calling filled with a NULL expect the "Cannot execute script" one. Create log functionality for only one possible error is not useful too. What I am thinking is just make a string containing the strerror
message and pass it into filename
argument. Actually I should say, cgi_error is not a very nice design, although it looks simple, but it assume every variable string should append in the end, which is not the case.
Print additional debug message to stderr when execution of script failed.