pkg / errors

Simple error handling primitives
https://godoc.org/github.com/pkg/errors
BSD 2-Clause "Simplified" License
8.2k stars 697 forks source link

Fix frame format %n to also check if fn is nil as in %s #148

Closed cstockton closed 6 years ago

cstockton commented 6 years ago

I think this is probably rare, but figured it should be guarded against just the same.

davecheney commented 6 years ago

Is it possible to write a test for this?

cstockton commented 6 years ago

Yes, made the pull a bit more noisy if that's okay. Though I noticed when doing so the old version didn't panic, it just prints an empty string. When I looked up runtime funcforpc I saw it can return nil and didn't think to look and see if the methods checked their receiver. So this change becomes a bit more contentious since a program that worked previously would stop working, and potentially have a new stack trace format. I'm neutral here I think, but would understand just closing this out to err on the side of caution.

davecheney commented 6 years ago

In that case I'm probably going to have to say no. I can't afford to break anyone.

cstockton commented 6 years ago

I understand feel free to leave it closed, just want to clarify before I head out that I meant to say "could stop working" instead of "would stop working". Only one specific type of program would break which I could only see realistically ever existing in a unit test, to break a program must:

So it's probably unlikely a program would break, but that means it's just as unlikely to benefit much. Might be nice to make the %v and %s flags consistent under the same Frame state in the future though if ever given the chance (like a go2 or something).