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

Add request info to standard error rescue #955

Merged
merged 3 commits into from Apr 22, 2016

Conversation

jf
Copy link
Contributor

@jf jf commented Apr 11, 2016

Error backtraces should really come with this context (makes pinpointing where we should look much easier). Would be open to some discussion about how best to do the printing. Puma::Events#unknown_error could handle it... but I'd have to pass in the env request object.

@evanphx
Copy link
Member

evanphx commented Apr 12, 2016

No, we don't touch STDERR directly here. You need to use @events.

@jf
Copy link
Contributor Author

jf commented Apr 12, 2016

Ok. Let me know if this is ok for you? I had to send @events the env as well (the alternative was to just specifically send env['REQUEST_METHOD'] and env['PATH_INFO']). I also collapsed the @stderr.puts calls into one, so it writes all the lines in one call.

string_block = [ "#{Time.now}: #{kind} error: #{error.inspect}" ]
end
string_block << error.backtrace
@stderr.puts string_block.join("\n")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer you just call @stderr.puts multiple times rather than make an array and then join it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer multiple calls too. Sorry for not elaborating; the only reason I did a single call is so that there wont be the possibility (?) of other output from other @stderr.puts calls being interleaved among these debug lines. Do you think this is a valid concern / makes sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any comments, @evanphx ? Would appreciate your input on this.

@evanphx evanphx merged commit 9ad34ec into puma:master Apr 22, 2016
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.

None yet

2 participants