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.

Sign up for GitHub

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

Jump to bottom

Additional debug message #29

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

Additional debug message #29

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions fcgiwrap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

cgi_error("502 Bad Gateway", "Cannot execute script", filename);

default: /* parent */
Expand Down