Skip to content

Conversation

Wwwsylvia
Copy link
Member

@Wwwsylvia Wwwsylvia commented Jul 4, 2025

What this PR does / why we need it:

  1. Optimize the error message for oras cp
  • Before:
    image

  • After
    image

  1. Optimize the error message from registry error response by removing "recognizable error message not found:", impacting oras login and oras ls
  • Before:
    image

  • After ("recognizable error message not found" is removed):
    image

  1. Improve error message for the "path traversal disallowed" error (resolving part of Improve error message for pushing/pulling files with absolute paths #1744)
    • Before:

image

  • After:

image

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1469

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

Wwwsylvia added 10 commits July 7, 2025 13:42
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Wwwsylvia added 9 commits July 7, 2025 14:58
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Copy link

codecov bot commented Jul 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.54%. Comparing base (90b42bd) to head (2638976).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1773      +/-   ##
==========================================
+ Coverage   85.43%   85.54%   +0.11%     
==========================================
  Files         137      137              
  Lines        5966     5993      +27     
==========================================
+ Hits         5097     5127      +30     
+ Misses        618      615       -3     
  Partials      251      251              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Wwwsylvia added 9 commits July 9, 2025 15:35
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
@Wwwsylvia Wwwsylvia requested a review from Copilot July 10, 2025 02:47
Copilot

This comment was marked as outdated.

Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
This reverts commit bbd116c.

Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
This reverts commit df2dd33.

Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with minor suggestions

Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
@Wwwsylvia Wwwsylvia requested a review from Copilot July 14, 2025 03:08
Copilot

This comment was marked as outdated.

Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
@Wwwsylvia Wwwsylvia requested a review from Copilot July 14, 2025 03:17
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves error messages across multiple ORAS commands to provide clearer and more user-friendly feedback. The changes focus on optimizing error message formatting for oras cp, registry error responses, and path traversal errors.

Key changes include:

  • Improved error message prefixes for copy operations to specify source/destination registry context
  • Removed the "recognizable error message not found:" prefix from registry error responses
  • Enhanced path traversal error messages with clearer user guidance

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
cmd/oras/internal/errors/errors.go Replaces TrimErrResp with ReportErrResp and adds UnwrapCopyError function
cmd/oras/internal/option/target.go Updates error handling method from Modify to ModifyError with improved logic
cmd/oras/internal/option/remote.go Updates method name and uses new ReportErrResp function
cmd/oras/internal/option/binary_target.go Adds comprehensive copy error handling with specific prefixes
cmd/oras/root/pull.go Enhances path traversal error with user-friendly recommendation
cmd/oras/root/push.go Unwraps copy errors to remove unnecessary context
cmd/oras/root/cp.go Preserves copy error context for proper prefix processing
cmd/oras/root/attach.go Unwraps copy errors to remove unnecessary context
test/e2e/internal/utils/const.go Removes EmptyBodyPrefix constant
test/e2e/suite/command/resolve.go Updates test to remove empty body prefix
test/e2e/suite/command/cp.go Updates test expectations for new error format
test/e2e/suite/auth/auth.go Restructures tests and adds new test for modified error prefix
cmd/oras/internal/option/target_test.go Updates all test method names from Modify to ModifyError
cmd/oras/internal/option/binary_target_test.go Adds comprehensive tests for copy error handling
cmd/oras/internal/errors/errors_test.go Adds tests for new ReportErrResp and UnwrapCopyError functions
Comments suppressed due to low confidence (2)

test/e2e/suite/auth/auth.go:37

  • The removal of the copy command test from this location should be verified to ensure equivalent test coverage exists elsewhere. The copy command appears to be covered by the new test below, but this should be confirmed.
			RunWithoutLogin("attach", ZOTHost+"/repo:tag", "-a", "test=true", "--artifact-type", "doc/example", "--registry-config", authConfigPath)

test/e2e/suite/auth/auth.go:63

  • The removal of push and pull commands from the invalid credentials test reduces test coverage. These commands should be tested elsewhere or the removal should be justified.
			RunWithInvalidCreds("attach", ZOTHost+"/repo:tag", "-a", "test=true", "--artifact-type", "doc/example")

Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Wwwsylvia Wwwsylvia merged commit 943ca76 into oras-project:main Jul 14, 2025
8 checks passed
@Wwwsylvia Wwwsylvia deleted the cp_err_refactor branch July 14, 2025 04:32
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 this pull request may close these issues.

improve error message of oras cp
3 participants