lnrpc/routerrrpc+routing: fix send to route error propagation

[Roasbeef]

In this commit, we fix a regression introduced by a recent bug fix in
this area. Before this change, we'd inspect the error returned by
processSendError, and then fail the payment from the PoV of mission
control using the returned error.

A recent refactoring removed processSendError and combined the logic
with tryApplyChannelUpdate in order to introduce a new
handleSendError method that consolidates the logic within the
shardHandler. Along the way, the behavior of the prior check was
replicated in the form of a new internal failPayment closure. However,
the new function closure ends up returning a channeldb.FailureReason
instance, which is actually an error.

In the wild, when SendToRoute fails due to an error at the
destination, then this new logic caused the handleSendErorr method to
fail with an error, returning an unstructured error back to the caller,
instead of the usual payment failure details.

We fix this by no longer checking the handleSendErorr for an error as
normal. The handleSendErorr function as is will always return an error
of type *channeldb.FailureReason, therefore we don't need to treat it
as a normal error. Instead, we check for the type of error returned, and
update the control tower state accordingly.

With this commit, the test added in the prior commit now passes.

Fixes #5477.

[Roasbeef]

Similar to ReportPaymentFail below, we just count this as an internal failure reason. This lets the method always return *channeldb.FailureReason.

The downside is that we lose this error context from the PoV, of the caller.

[yyforyongyu]

I'd prefer the internalErrorReason. To me errors come from other packages/services do seem to be "internal" when we are viewing it as the user of the routing. It's probably better (haven't thought through it) though we could wrap the error inside the internal error and returns it back to the caller.

[Roasbeef]

I think the method should maybe just return *channeldb.FailureReason directly so it's clear that it can't return a "real" generic error.

[yyforyongyu]

nit: there's a typo in the commit message,

As is, this test will fail as a recent refactoring causes us to return a
channeldb.FailureReason error, since the newly added handleSendError
code path in the SendToRoute method will return the raw error, ratehr rather
than the shardError, which is of the expected type.

[yyforyongyu]

Thanks for the fix! LGTM, just a few nits.

I haven't dug into the details about how errors are handled here, just a thought. It's probably better if we could make package-specific error types so that the raw error is never returned. Maybe a errors.go under every package, and when returning an error inside the package, it must use the defined types and no more if err != nil { return err}. This would have the effect of forcing us to look into the errors returned and assign the appropriate type, so error-type-aware...

[yyforyongyu]

I'd prefer the internalErrorReason. To me errors come from other packages/services do seem to be "internal" when we are viewing it as the user of the routing. It's probably better (haven't thought through it) though we could wrap the error inside the internal error and returns it back to the caller.

[Roasbeef]

It's probably better if we could make package-specific error types so that the raw error is never returned.

Yeah generally we have this, like the set of errors in routing/errors.go. The issue here however, was the ambiguity w.r.t if an error was actual a fatal error, or an "internal error" to eventually be passed up and/or ignored.