Fix bad error response when manifest is stripped

[mauricefisher64]

Changes in this pull request

Fix bad error code when manifest is stripped and XMP reference is present

Checklist

• This PR represents a single feature, fix, or change.
• All applicable changes have been documented.
• Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.

[gpeacock]

The idea behind the RemoteManifestUrl error is that we are returning the URL so that it can be fetched externally when fetch_remote_manifest is disabled. So we don't want to add extra text to the url in that case, just return the url.

[codecov-commenter]

Codecov Report

Merging #153 (11daadb) into main (42cb1c0) will increase coverage by 0.05%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
+ Coverage   78.27%   78.33%   +0.05%     
==========================================
  Files          67       67              
  Lines       14684    14698      +14     
==========================================
+ Hits        11494    11513      +19     
+ Misses       3190     3185       -5     

Impacted Files	Coverage Δ
sdk/src/store.rs	85.26% <93.33%> (+0.38%)	⬆️
make_test_images/src/make_test_images.rs	67.78% <0.00%> (-0.42%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[mauricefisher64]

Well, this case is different. The reason this failure is that it is a remote url but the feature is disabled. What to you want to return here?

[gpeacock]

There are two remote errors defined as follows:

#[error("could not fetch the remote manifest")]
RemoteManifestFetch(String),

#[error("must fetch remote manifests from url")]
RemoteManifestUrl(String),

The first should be used when we have a valid url, try to fetch it and fail.
The second is for this case where fetching is disabled, so the caller will need to fetch the URL externally.

[gpeacock]

On RemoteManifestUrl, it is important that only the URL itself is returned so that the caller can read it and attempt to fetch the URL on their own.

[dkozma]

Should there be a test case for this? Also, when testing on my side, I saw it try to fetch a local JUMBF url as if it was a remote URL - is this expected?

[mauricefisher64]

Should there be a test case for this? Also, when testing on my side, I saw it try to fetch a local JUMBF url as if it was a remote URL - is this expected?

You must have an old version. That was changed in a prior PR.

[gpeacock]

looks good. I'm not crazy about adding yet another jpeg just to test this case, But it is a good idea to have a test case for it. Perhaps when I get around to generating the images for unit tests, I'll add one to generate this case.