Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

make sure all caught panics have a stack with them #1176

Merged
merged 4 commits into from
Jul 16, 2018

Conversation

markbates
Copy link
Member

No description provided.

@markbates markbates added this to the v0.12.4 milestone Jul 16, 2018
@markbates markbates self-assigned this Jul 16, 2018
@@ -58,12 +58,12 @@ func (a *App) PanicHandler(next Handler) Handler {
if r != nil { //catch
switch t := r.(type) {
case error:

Choose a reason for hiding this comment

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

you might have to do err = t otherwise err is nill still

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch!

@codecov
Copy link

codecov bot commented Jul 16, 2018

Codecov Report

Merging #1176 into development will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           development   #1176   +/-   ##
===========================================
  Coverage         51.1%   51.1%           
===========================================
  Files               76      76           
  Lines             3835    3835           
===========================================
  Hits              1960    1960           
  Misses            1757    1757           
  Partials           118     118
Impacted Files Coverage Δ
errors.go 66.66% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c7507a...f72a0b9. Read the comment docs.

@markbates
Copy link
Member Author

@marwan-at-work fixed! and added some tests to make sure. thanks for the review!

marwan-at-work
marwan-at-work previously approved these changes Jul 16, 2018
Copy link

@marwan-at-work marwan-at-work left a comment

Choose a reason for hiding this comment

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

@markbates thanks. This definitely takes care of the nil error. However, Buffalo still doesn't print the stack in the log. This is probably related to #1171

fyi this is what i got

ERRO[2018-07-16T12:43:20-04:00] runtime error: invalid memory address or nil pointer dereference content_type="*/*" duration="495.828µs" human_size="0 B" method=GET params="{}" path=/hem request_id="4b361f3b89-f0c6aa9aff" size="0" status="0"

@markbates markbates merged commit 04fee7c into development Jul 16, 2018
@markbates markbates deleted the panic-handler-with-stack branch July 16, 2018 20:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants