failure
failure copied to clipboard
context() and with_context() are misleading names
I think .context()
and .with_context()
are misleading when reading code using them. They appear to add context to the operation, but they only add information to the error case.
I suggest renaming them to .err_context()
and .with_err_context()
or something similar to make the code more expressive.
I'm open to deprecating context and replacing with a new name that more clearly implies that they are error related. However, I have these constraints:
- I don't like using
err_
to distinguish things. I think its much better to come up with a natural name than to take the sort of shortcut route of just adding the variant as part of the method name. - I would prefer it to have the same name in
ResultExt
as it does inFail
.
I think of .context()?
as being an operation like .unwrap()
and .expect()
; that is, a widely understood idiom. So I'd like to find some single word similar to context, but which more clearly implies that its on the error side of things.
I understand the reasoning behind your constraint.
I am not an english native speaker, so I am not sure I can come up with a good suggestion, but I would like to share my point of view regarding the examples you bring.
I like .unwrap()
as it's clear that there will be a result in both cases (Ok
and Err
): i fit was Ok
, I get the value; if it was Err
I get an error (a panic, to be precise, but nonetheless an error).
OTOH I find .expect()
one of the most confusing "keywords" in rust. When I see do_something().expect("something else")
I expect (pun intended) that the do_something()
will return the value something else
, and to get an error if it doesn't. Basically: it's for tests only, and my intuitive guess is wrong. .or_panic(msg)
would have been a better, IMO.
The same happens with .context()
, in my mind, and that's why I opened the issue.
When I see do_something().context("some text")
I think "Ok, this operation is going to use the context I pass for something, like logging or tracking". I absolutely don't think about error wrapping.
With .map_err()
or .chain_err()
it's extremely clear it's only applied to the error case.
An alternative suggestion could be .or_bail()
but I'm not completely sold on this one either, as it's not clear the error is going to be wrapped/linked/contextualised.
I suggested .err_context()
because in my mind it made extremely clear what was happening. I am not attached to it.
My only concern is that it should be clear while reading the code what is happening, without having to know the internals of the crate being used.
I think Context
shouldn't really be called Context
but Chain
(and as such the names of context
and with_context
would also change).
I recently changed a not so small crate from a monolithic error_chain to failure
using different patterns of
errors and error composition depending on each errors usage. My experience was mainly good, but
I think Context
is bad name, through I know I can understand where the name comes from: you add
a "context" in form of e.g. a error kind saying which operation failed to the error.
But most (all?) of this context could be seen as "light wight errors" and as such it can be seen as a form of error chaining.
When I speak about light wight errors I mean errors which have no cause, no backtrace and
optimially are copy or at last cheap to clone. If represented as a trait it would be
trait LightFail: Display + Send + Sync + 'static {}
(+ Clone
?) which just happens
to be the required constraint for values used as a context in Context
.
The problem is that there are many kinds of context's and Context
only works for with a very
specific kind of context's mainly (not solely) such which you could also see as "light wight errors"
like e.g. error kind's &'static str
's which can be seen as stringly typed light wight errors etc.
But there are a lot of other context, e.g. a absolute or relative position in a source where the failure originated from, a snapshot of the configuration for the failed operation, etc.
While many but not all of them might implement Display
. Even this which do should often not be displayed alone, e.g. a message like hy "tom"
makes little sense without displaying the error (e.g. NoQuotingAllowed => no quoting allowed, quoting appeard in message: "hy \"tom\""
).
This would also resolve the confusion wrt. the utility methods of ResultExt
and Fail
. I more then
once accendentally started typing chain
only to correct it to context
, because I was thinking about
chaining errors (through that might just be me ;=) )
If Context
get's renamed to Chain
, then .context(kind)
could become .chain(kind)
and .with_context(|e| kind)
could become .chain_with(|e| kind)
. I will make another post wrt. to the naming.
Expect what I already mentioned about context
another think I think could be improved if the name is changed anyway is to not use a with_
prefix.
All places in rust where a function has a variant accepting a value and one accepting a closure it is
denoted by a suffix e.g. or
=> or_else
, and
=> and_then
, etc.
Sure, context_with
/context_then
sound a bit stupid and context_from(func)
might also not be the best, but if context is changed anyway we might just try to fix this too.
Also all "in-the-wild" use cases of a with_
prefix I stumbled over where functions creating a
new value by consuming the value and combining it with another value, but they where not
that common an I would have to search for a good example (outside of code written by me, I
mean using my own code as example would be pointless right ;) ).
I completely agree with @Alex-PK, both .expect()
and .context()
are misleading for me. .chain()
is still a too broad term since you apply it to the Result which may be either Ok
or Err
.
Any of the following sound good to me:
-
.map_err()
/.map_err_with()
(but it is already taken by the standard Result implementation) -
.chain_err()
/.chain_err_with()
-
.wrap_err()
/.wrap_err_with()
(as opposed to.unwrap()
) -
.map_failure()
/map_failure_with()
(too long?)
I seem to prefer .wrap_err()
, but I find all three to express the intention better than the current .context()
API.
Other options I have generated:
-
.or_wrap()
/.or_wrap_with()
(it has some connection to.unwrap()
, but it does not reflect the intent clearly to me) -
.or_failure()
/.or_failure_with()
(too long and still doesn't sound right to me)
P.S. I noticed that wrap_err
may sound like "wrapper", map_err
-> "mapper", chain_err
-> "chainer", and while it is an unintended pun, it may be still confusing.
@frol I agree that .chain()
is a bit to general for the function name,
main main concern was to change failure::Context
to failure::Chain
, which I think isn't to general as for example error are also often called Error
instead of something like CrateNameError
.
I post the idea to rename the type here instead of another issue as both name changes can be seen as one change in naming.
I don't think a or_
prefix is appropriate as they are used in "if this fail try to use that" manner, e.g.
Result.or
accept another Result
and Option.or
accepts another Option
.
I also prefer the usage of the word chain
over wrap
in this context, as you don't just wrap the failure,
you create a data structure which is extremely similar to a linked list, it has a "head" (the think currently
referred to as the context
) and a "tail" (which is Either<Backtrace, Error>). Sure it's not a linked list,
as it has backtraces attached, erases the type etc. that's why I call it a Chain
(the other reason is, that
it would be less confusing for people migrating from error_chain
).
So chain_err
?
"On" has been used for error handling for 40 years! OnError: ifError
on_error on_error_wrap on_error_report on_error_inform on_error_log on_error_do {}
or perhaps use fail/failure
on_fail on_fail_do {} on_fail_report on_failure_report on_failure_wrap fail_with( a_context )
on_failure wrap_failure_with