Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

Obtaining only message #93

Open
gudmundur opened this issue Nov 13, 2016 · 15 comments
Open

Obtaining only message #93

gudmundur opened this issue Nov 13, 2016 · 15 comments

Comments

@gudmundur
Copy link

Is it possible to get the message passed to errors.Wrap(err, message)? What I specifically mean is message itself, without anything from err.

My use case is that I wrap underlying errors, and pass in a human friendly message, that I would like to expose to customers via our APIs. Currently I'm parsing the message out by splitting by : , which is brittle.

@groob
Copy link

groob commented Nov 13, 2016

Can I suggest an alternative? Have your errors implement a specific interface, and then when you respond to the client check if the error satisfies that interface and respond appropriately.

Here's a short example of what I mean: https://play.golang.org/p/xpl-71EKbj
And one a bit longer checking for different errors. https://gist.github.com/groob/ff49f470eeb389c7aa237e667beab326

@davecheney
Copy link
Member

Maybe errors.Wrap is not what you want.

It feels like you are trying to create a side channel to send extra data related to the path the code took to the site the error was generated. You may be better served by writing your own error type.

See also #34

@jasonish
Copy link

I found myself wanting this as well. In my case I was wrapping my errors with an additional message. My http service has a single point of logging errors, and to avoid sending too much information (some that may be private) to the http client, I'd like to just extract the friendly the message as well, but log the stack trace.

So I can do it using my own event type, but then I'm supply 2 messages which is redundant. Of course I could be doing it all wrong as well, but in my case a Wrap without requiring a message could be an alternative.

@davecheney
Copy link
Member

A wrap without a message is called WithStack. Is that useful for your use case?

@jasonish
Copy link

A wrap without a message is called WithStack. Is that useful for your use case?

Yes, sorry for the noise, I should have dug a bit deeper first. Just tried this out for my use case and it gives me exactly what I need. Thanks.

@tehmoon
Copy link

tehmoon commented Dec 8, 2017

I've been in the same situation as @gudmundur. I love this package, I'm using it in all my project. So first, thank you.

My use case is the following:

var ErrNotFound = errors.New("")
func fn(f string) (error) {
  return errors.Wrapf(ErrNotFound, "The file %s has not been found", f)
}

func main() {
  err := fn("/nonexistant/blah")
  if err != nil {
    log.Fatal(err.Error())
  }
}

I'm using ErrNotFound just to contextualize the error, meaning I can later do error.Cause() to get the underlying error and match it to ErrNotFound variable.

What I found too bad is that doing var ErrNotFound error which will set it to nil, Wrap() -- or others -- will return nil causing the error to never enter the condition.

What about treating Wrap(nil, "message").Error() like if "message" was the latest message's hence won't have the last :?

I can try to come up with a PR if you think that's a good idea.

@davecheney
Copy link
Member

davecheney commented Dec 8, 2017 via email

@tehmoon
Copy link

tehmoon commented Dec 9, 2017

Hi @davecheney thank you for answering this fast!

Yes you are right, that's what I ended up using in most cases. But I am still leaning towards treating the root cause as a variable that I could compare to something without having to declare a new struct every time I have a new "simple" error.

I really like for example the "io" package where you can simply compare the error interface against the variables that are declared in the package. But I loose the "printf" ability though and was trying to find a compromise since I didn't want to return the message from the underlying error but from where it came last.

I'm also sure that I'm doing things wrong, I'll try to look at some code to really understand how to do errors. I actually discover your package through your blog entry: https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully which totally helped me a lot already.

@davecheney
Copy link
Member

davecheney commented Dec 9, 2017 via email

@tehmoon
Copy link

tehmoon commented Dec 9, 2017

Ok I'll think about it more and give it a try, maybe with a new approach. Thank you so much for taking the time.

@davecheney
Copy link
Member

davecheney commented Dec 9, 2017 via email

@puellanivis
Copy link

Yes, I have had a few people tell me, “but the standard library does XY” and I’m like, well… it was written in an earlier time, and views have changed about the best way to do XY now…

Of course, I still think the “oob” conditions (io.EOF) being applied in the error namespace is the biggest example of this, with option applications second.

@bjohnson-va
Copy link

Jumping in here because I haven't seen anyone mention my use-case.

I would find being able to access the message useful for testing. i.e:


func myFunc(thing3Helper helper) (error) {

  doThing1()
  doThing2()
  err := doThing3(thing3Helper)
  if err != nil {
    return errors.Wrap(err, "Failed to do thing 3")
  } 

}
func TestReturnsCorrectErrorIfThing3Fails() {
  helper := helperThatFails{}
  err := myFunc(helper)
  assert.Equal(err.Message(), "Failed to do thing 3")
}

It would be nice to be able to treat doThing3 as a black box and not need to know what its errors look like.

Would this be a valid case, or am I misusing this library?

@puellanivis
Copy link

I think in this case you are attempting to fit a square peg in a round hole.

If you wish to treat doThing3 as a black box, then you should be testing doThing3 directly, rather than in the composed myFunc.

Testing that myFunc returns an error in the case of doThing3 returning an error need only be tested insofar as myFunc will return a non-nil error when doThing3 would return an error. The form of the error itself is otherwise irrelevant. (Unless you have documented it to be relevant.)

@bjohnson-va
Copy link

bjohnson-va commented Jul 12, 2018

Yeah, what you've described is the option I've settled for. My only qualm with it is that, in the non-contrived version of this scenario, I actually have multiple "things" that may error. So, I feel like it would be nice to assert that the correct error was raised, rather than just an error.

That said, if I were truly blackbox testing myFunc, I wouldn't have any reason to expect a specific error¹. And, actually, checking error.Cause(...) would probably be more appropriate given that I'm passing in the helper that generates the "causal" error.

¹ Unless I were doing TDD to prescribe the errors that my function returns.

--

Edit: I've since decided that this is the wrong approach.

From the standpoint of my test, all I should know about the function-under-test is:

  • the stub I've provided will return a specific error
  • my function should return a new error whose Cause is that stub-error

If I have multiple "things" that are all calling the same function on the stub, I might need to build a more advanced stub that is capable of handling many input values and only erroring for the ones that match the scenario I'm testing. Anything more than that, and I'm creeping into whitebox testing.

Also, the function under test should be free to modify its error messages without breaking the test.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants