API design question: should a 407 from a proxy check be Ok or Err?

[u/GG_Men]

I'm working on a Rust SDK that has a small check() method for testing a proxy connection, and I'm not sure I'm using Result in the least surprising way here.

If the proxy replies with 407 Proxy Authentication Required, I currently return Ok(Check), not Err.

My reasoning is basically: the check itself worked) I connected to the proxy and got a real response back. For this method, the status/reason/headers are useful information.

Err is only for cases where I couldn't get a usable response at all - timeout, DNS failure, connection failure, malformed response, etc.

Roughly:

match proxy.check(connect)? {
    check if check.status == 200 => {
        // accepted
    }
    check if check.status == 407 => {
        // proxy replied, auth rejected
    }
    check => {
        // some other resp
    }
}

The part that feels a bit weird is this:

let check = proxy.check(connect)?;

That kind of reads like "the proxy is fine", while what it really means is only "the proxy gave a usable response".

Would you expect 407 to be an Err here, or does treating a protocol-level refusal as a successful diagnostic result make sense? 🙊

[u/BobTreehugger]

Both are valid depending on the use case. Since you're returning the check struct that has the http status, that's probably fine.

Not sure if you want to do it, but a nice thing about Result is that you can nest them -- Result<Result<Check, AuthError>, ConnectionError> is a valid thing to do, but you know more about the intended use of this than I do.

If this is mostly used to check if you have a valid connection, your original design seems better, if you mostly want to check if you can actually auth and go through the proxy, seems like you want error if you can't auth.

let check = proxy.check(connect)?;

Looks to me like you were able to check, not what the result of the check is.

[u/GG_Men → u/BobTreehugger]

Yeah, that last sentence is basically what I had in mind. check()? means the check ran and I got a usable response, not that auth necessarily succeeded.

I think I’ll keep the status in Check then. Nested Result is neat, but probably more ceremony than this needs)

[u/thorhs → u/GG_Men]

You could rename the function to check_connection, or check_without_auth. Could then also have a check_with_auth to validate the auth method, if needed. This makes the intent more explicit.

[u/teerre]

Return Check where it failed would be surprising to me. Checking error codes manually is unidiomatic unless you're working a literal http library

``` enum ProxyError { Auth, Connection }

Result<Check, ProxyError> ```

Seems like the natural shape. The only reason to separate the error would be if from Check you can also authenticate, but that would be two calls and some nesting, probably not worth it

It also highly depends what are users expected to do with these errors. The more actionable the error is, the larger its api can be

[u/GG_Men → u/teerre]

Keeping a valid proxy response in Ok still makes sense to me, but your point about callers having to interpret raw status codes is fair)

Maybe the better shape is for Check to expose a higher-level outcome like AuthFailed, while keeping transport/check failures in Err.

[u/owbrooly]

It s a bit tricky. I m guessing you are working on http proxy, in which case 407 when no auth is supplied indicates that proxy requires auth. But if authentication was supplied it indicates that auth failed. Both errors are user facing, so I guess best way to handle this is to return Enum in Ok() branch that minimally represents current state

[u/GG_Men → u/owbrooly]

That’s a good point. I hadn’t really separated “auth required” from “auth failed” when thinking about the 407 case.
Keeping protocol responses in Ok() still feels right to me, but maybe an outcome enum inside Check could make that distinction explicit without changing the outer Result

[u/owbrooly → u/GG_Men]

Yeah, that feels right. Err is more of something went wrong along the way in this case. At least it s how I would feel about it

[u/South_Survey_2088]

I would argue that the function name is the issue here. "check()" leaves a lot of ambiguity and reads more like a boolean-like. I am not a web dev so there might be a better name, but something like status() would make it clearer that the result is a state that should be inspected rather than a condition.

[u/Lucretiel]

Often for cases like this I start wrapping Results in each other. For example, in low level network protocol code, I often end up with Result<Result<Response, ProtocolError>, TcpError>. I’d recommend something like that.

That being said, I think you’re right to consider this design space. The primary advantage of structured errors and errors-as-values is that you can react to them in granular ways; you should absolutely not feel obligated to unthinkingly propagate all errors to callers. If a 407 code semantically represents success for the particular operation you’re attempting, then by all means return it as an Ok. 

[u/maccam94]

The one reason I'd consider making this an error is for ? ergonomics. Then happy path just gets the value, and anything else propagates up to the error handling/retry logic.

[u/neon_lodger]

agreed on the naming issue. If you call it status() and return a Result<Status, ConnectionError>, then 407 is clearly Ok(Status::NeedsAuth). This separates "the proxy is dead" from "the proxy is talking to me but rejecting my credentials," which keeps your ? operator honest since a hard fail still triggers early exit.

[u/stevecooperorg]

generally I'd put;

- OK as "we had the conversation" even if we don't like the answer.

• Err as "we couldn't connect / there was a timeout / etc"

The problem is naturally two-stage -- make the request / interpret the response. Return OK if you made the request. Handle "I don't like it" later. So;

match request()? { 
  Ok(code) => match code { 
    NOT_FOUND => { 
//      seems bad, right? but we successfully 
//      asked the question; we just don't like the answewr.