Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ErrorInfo detail field is object in registry responses #59

Closed
petkovicdanilo opened this issue Aug 31, 2021 · 4 comments · Fixed by #70
Closed

ErrorInfo detail field is object in registry responses #59

petkovicdanilo opened this issue Aug 31, 2021 · 4 comments · Fixed by #70

Comments

@petkovicdanilo
Copy link
Contributor

Although spec says that detail field in ErrorInfo is string with unstructured data it seems that all registries make detail field an object.

For example, Dockerhub returns this for unknown manifest:

{
    "errors": [
        {
            "code": "MANIFEST_UNKNOWN",
            "message": "manifest unknown",
            "detail": {
                "Tag": "lates"
            }
        }
    ]
}

Quay:

{
    "errors": [
        {
            "code": "MANIFEST_UNKNOWN",
            "detail": {},
            "message": "manifest unknown"
        }
    ]
}

Microsoft's MCR :

{
    "errors": [
        {
            "code": "MANIFEST_UNKNOWN",
            "message": "manifest tagged by \"lates\" is not found",
            "detail": {
                "Tag": "lates"
            }
        }
    ]
}

This results in failure to deserialize any of the above ErrorResponse structs.

Is this the problem of registry implementers, this library or the spec itself?

@Furisto
Copy link
Member

Furisto commented Aug 31, 2021

Considering that the exact wording of the spec is

The detail field is OPTIONAL and MAY contain arbitrary JSON data providing information the client can use to resolve the issue

the most correct (but not the most convenient) way would be to expose a Json type like this:

pub enum Json {
    Null,
    Bool(bool),
    Number(f64),
    String(String),
    Array(Vec<Json>),
    Object(Map<String, Json>),
}

and then you have to match to know what you got. Serde already defines such a value but we should probably define our own, because it would be visible in our public api and so on...on the other hand we could just take care that the detail field can be serialized to a string and then it is up to the consumer to handle it. Thoughts?

@petkovicdanilo
Copy link
Contributor Author

I don't know which approach is the best to go with, but I could implement and submit pull request for whichever turns out to be the best.

@Furisto
Copy link
Member

Furisto commented Sep 6, 2021

@petkovicdanilo Then I suggest we do the simple thing and ensure that the detail field is properly deserialized as a string. Do you want do it?

@petkovicdanilo
Copy link
Contributor Author

Yeah, sure. I can try to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants