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

Additional debug message #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

w32zhong
Copy link

@w32zhong w32zhong commented Jul 2, 2015

Print additional debug message to stderr when execution of script failed.

@w32zhong
Copy link
Author

w32zhong commented Jul 2, 2015

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));
Copy link
Contributor

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));

Copy link
Author

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

Copy link
Contributor

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?

Copy link
Author

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.

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.

2 participants