URL in Problem.Type doesn't exist

[bartekn]

// Inflate expands a problem with contextal information.
// At present it adds the request's id as the problem's Instance, if available.
func Inflate(ctx context.Context, p *P) {
    //TODO: add requesting url to extra info

    //TODO: make this prefix configurable
    p.Type = "https://stellar.org/horizon-errors/" + p.Type

    p.Instance = requestid.FromContext(ctx)
}

https://github.com/stellar/horizon/blob/08885e0bae6fb2eb5ecece05483cbae0d35816e7/src/github.com/stellar/horizon/render/problem/main.go#L58

[bartekn]

Also, looks like horizon.Client depends on Type field when returning result codes:

// ResultCodes extracts a result code summary from the error, if possible.
func (herr *Error) ResultCodes() (*TransactionResultCodes, error) {
    if herr.Problem.Type != "transaction_failed" {
        return nil, ErrTransactionNotFailed
    }

https://github.com/stellar/go/blob/411d5554e6d7cbb5b699f13aacf2a87d7a674b47/clients/horizon/error.go#L40

We need another field that will only contain error code - without URL to an error.

[jedmccaleb]

I'd vote for dropping the URL

[nullstyle]

@jedmccaleb moving away from the url wouldn't be compliant with https://tools.ietf.org/html/rfc7807

I'm a bit miffed that our first instinct is that the server is designed wrong in this situation when it looks to me like the client never worked for this scenario.

---

The nice thing about using a URL is that it should be resolvable to the docs for that error. We haven't yet done that work, but we probably should. I mean, I'm fine with us scrapping HAL and Problems and doing something entirely on our own if you guys think it will make things easier for devs, but let's not pretend like its going to solve the situation that gave rise to this problem... the docs even include an example using the full url: https://github.com/stellar/horizon/blob/master/docs/reference/errors/transaction-failed.md

[bartekn]

I'm a bit miffed that our first instinct is that the server is designed wrong in this situation when it looks to me like the client never worked for this scenario.

I agree, that's is why I suggested:

We need another field that will only contain error code - without URL to an error.

According to specification, it's possible to extend problem object: Extension Members. My main concern with using URL in ResultCodes is that it may change in a future (there is even a comment suggesting this: //TODO: make this prefix configurable). We may also switch to using Title field.

---

The other thing is that the code in stellar/go was not reviewed and was not tested. I think we need to 1) disallow pushing directly to master branch in all of our repos 2) require at least 1 review from other team member. This can be easily switched in GitHub settings.

[nullstyle]

According to specification, it's possible to extend problem object: Extension Members.

If we're just going to use the "Do whatever you want" features of the spec to avoid using the spec how it was intended (the type URI is the identifier to switch on) then IMO we should just do our own thing.

My main concern with using URL in ResultCodes is that it may change in a future (there is even a comment suggesting this: //TODO: make this prefix configurable).

They won't be changing. We need to implement redirects at those URLs to redirect into the documentation.

We may also switch to using Title field.

nonono :) "title" is a human readable field, we shouldn't be coding against it. A client would render the title to the user to explain why their request failed:

"title" (string) - A short, human-readable summary of the problem type. It SHOULD NOT change from occurrence to occurrence of the problem, except for purposes of localization (e.g., using proactive content negotiation; see [RFC7231], Section 3.4).

---

The other thing is that the code in stellar/go was not reviewed and was not tested. I think we need to 1) disallow pushing directly to master branch in all of our repos 2) require at least 1 review from other team member. This can be easily switched in GitHub settings.'

I'm cool with that, but this isn't the root problem IMO... We're trying to do too much on too many different projects without enough collaboration and shared code. this specific issue was "Hey, we've got this thing that looks like a horizon client in the bridge server, why don't we just import it into the monorepo minimally so others can use it?" We chose the quick solution because we didn't have enough resources to choose the correct solution.

[bartekn]

My main concern with using URL in ResultCodes is that it may change in a future (there is even a comment suggesting this: //TODO: make this prefix configurable).

They won't be changing. We need to implement redirects at those URLs to redirect into the documentation.

Then OK, EOT.