Proposal Details

Basically, https://go-review.googlesource.com/c/crypto/+/681037 because it helps understand the issues the ACME server encountered with the order when that errors.

It also better implements RFC 8555 by making this more accessible and easier to introspect as well since all errors from an ACME compliant server should have a problem description which an Error already helps describe and which we lose from an OrderError.

Comment From: gabyhelp

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

Comment From: rolandshoemaker

This explicit proposal here is:

type OrderError struct {
    ...
    Problem  *Error
}

Comment From: aclements

Seems straightforward enough.

Should OrderError have an Unwrap method that returns Problem?

Could you write a doc comment for the Problem field? (Or an updated comment for OrderError)

Comment From: aclements

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — aclements for the proposal review group

Comment From: rolandshoemaker

Seems straightforward enough.

Should OrderError have an Unwrap method that returns Problem?

I'm not entirely convinced adding Unwrap to OrderError would be all that useful. We explicitly document that OrderError is returned from WaitOrder, and if you're not inspecting the error directly at the WaitOrder call site, you likely need the additional context in the OrderError (Status, OrderURL) to understand the resulting Error.

Could you write a doc comment for the Problem field? (Or an updated comment for OrderError)

type OrderError struct {
    // Problem is the error that occurred while processing the order.
    Problem  *Error
}

The ACME spec makes the error returned from the order API optional, but it is expected to be populated with a negative order status. Unfortunately this is not made entirely clear in the specification (), so it is plausible that an implementer could return an invalid status without a error. This seems unlikely, but leaving Problem as a pointer is probably still a good idea.

Comment From: aclements

Based on the discussion above, this proposal seems like a likely accept. — aclements for the proposal review group

The proposal is to add the following field to x/crypto/acme.OrderError:

type OrderError struct {
        // Problem is the error that occurred while processing the order.
        Problem  *Error
}

Comment From: aclements

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — aclements for the proposal review group

The proposal is to add the following field to x/crypto/acme.OrderError:

type OrderError struct {
        // Problem is the error that occurred while processing the order.
        Problem  *Error
}

Comment From: gopherbot

Change https://go.dev/cl/681037 mentions this issue: acme: include order problem in OrderError

Comment From: cpu

Thanks @sigmavirus24 & @rolandshoemaker for seeing the proposal through :bow:

I revived my earlier CR w/ an implementation: https://go-review.googlesource.com/c/crypto/+/681037