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.

Sign up for GitHub

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

Jump to bottom

improve the returned error of oci.NewFromTar when the passed-in path is not a tarfile #640

Closed
qweeah opened this issue Nov 7, 2023 · 3 comments · Fixed by #799 · May be fixed by #793
Closed

improve the returned error of oci.NewFromTar when the passed-in path is not a tarfile #640

qweeah opened this issue Nov 7, 2023 · 3 comments · Fixed by #799 · May be fixed by #793
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone
future

Comments

@qweeah
Copy link
Contributor

qweeah commented Nov 7, 2023 •
edited
Loading

oci.NewFromTar can be used to create a new read-only OCI store from a tarfile path. If the passed-in path doesn't point to a tarfile, currently(v2.3.1) below errors will be returned:

  • If the length of the file is zero, a failed to open OCI layout file: oci-layout: file does not exist error will be returned
  • If the length of the file is not zero, an io.ErrUnexpectedEOF will be returned

As a user of oras-go SDK, I would like this function to return a unified and exported error type. Also, the error should indicate that the passed-in path doesn't point to an expected tarfile.
@yizha1 yizha1 added enhancement New feature or request good first issue Good for newcomers labels Nov 7, 2023
@yizha1 yizha1 added this to the future milestone Nov 7, 2023
@ADorigi ADorigi mentioned this issue Jul 21, 2024
Open
@ADorigi
Copy link

ADorigi commented Jul 21, 2024

Hello

I would like to work on this issue.

I have tried implementing the required code changes in the PR #793 as best I could interpret them.
Please let me know if any changes are required.

@shizhMSFT
Copy link
Contributor

@ADorigi Thanks for your contribution. After discussing with @qweeah offline, this issue is no longer valid.

The more user-friendly error messages should be handled in the CLI instead of in the oras-go SDK.

A proper example of oci.NewFromTar error handling would be

store, err := oci.NewFromTar(ctx, path)
if err != nil {
  return fmt.Errorf("%s: invalid or corrupted oci-style tarball: %w", path, err)
}

@qweeah
Copy link
Contributor Author

qweeah commented Jul 25, 2024 •
edited
Loading

Thanks @ADorigi for the contribution. io.ErrUnexpectedEOF already implies corrupted(invalid) tar ball file.

Also, we notice that failed to open OCI layout file: oci-layout: file does not exist got an extra space. It's caused by missing operation in the path error. Will raise a PR to fix that.

@qweeah qweeah mentioned this issue Jul 25, 2024
Merged
Wwwsylvia pushed a commit that referenced this issue Jul 30, 2024
@qweeah
fix: add missing operation to fs path error (#799)
bacf8f0 
When opening a folder as OCI image layout and there is no `oci-layout`
file exists, the error would be

> failed to open OCI layout file:  oci-layout: file does not exist

There is a duplicated space in the error string, because the `Op` field
is missing in the returned `fs.PathError`. With fix in this PR, the
return error would be
> failed to open OCI layout file: open oci-layout: file does not exist

Fixes #640

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
future
4 participants
@qweeah @shizhMSFT @ADorigi @yizha1