zowe / zowe-cli

Zowe CLI
Eclipse Public License 2.0
113 stars 87 forks source link

standardizing error handling #2062

Closed ATorrise closed 7 months ago

ATorrise commented 7 months ago

What It Does Brings standard error handling from zosjobs to zosfiles

How to Test

Review Checklist I certify that I have:

Additional Comments

Output for zowe jobs submit lf noexist

Old Handling

image

New Handling:

image

Output for zowe jobs submit lf noexist

image

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 88.23529% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 91.08%. Comparing base (71fb5e8) to head (2afdebc). Report is 2 commits behind head on next.

Files Patch % Lines
.../src/zosfiles/upload/ftds/FileToDataSet.handler.ts 78.94% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## next #2062 +/- ## ========================================== - Coverage 91.09% 91.08% -0.02% ========================================== Files 638 638 Lines 18840 18837 -3 Branches 3924 3917 -7 ========================================== - Hits 17162 17157 -5 - Misses 1677 1679 +2 Partials 1 1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gejohnston commented 7 months ago

I am a little confused about the objective of the story itself.

A user specifies a non-existent file and the error message implies that the command has an internal logic failure. It blames the command handler for the error and displays a stack trace.

Should we instead capture the root error message 'no such file or directory' and display it in a format similar to the more user friendly format utilized for REST request errors that we used in a pull request several months ago?

gejohnston commented 7 months ago

Note that the current full zos-files error message is:

zowe files upload ftds local remote
Unable to perform this operation due to the following problem.
Node.js File System API error

Response From Service
Error: ENOENT: no such file or directory, lstat 'C:\ourstuff\records\Broadcom\zowe\tech_info\config_files\local'

Diagnostic Information
Error: ENOENT: no such file or directory, lstat 'C:\ourstuff\records\Broadcom\zowe\tech_info\config_files\local'

The format above was created in response to another issue about unfriendly error messages.

Perhaps we should make the zos-jobs messages adhere to the format used by zos-files?

sonarcloud[bot] commented 7 months ago

Quality Gate Failed Quality Gate failed

Failed conditions
10.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

adam-wolfe commented 7 months ago

After looking at this some more it seems like the best thing to do would be to not show a stack trace for the file not found error. Currently, in the latest versions of V2 and V3, the jobs submit lf command still displays a stack trace. I think, for v3, we would want the jobs command failure to match the zowe files upload error message format like Gene suggested.

gejohnston commented 7 months ago

The previous related issue is https://github.com/zowe/zowe-cli/issues/935 The previous related PR is https://github.com/zowe/zowe-cli/pull/1738

t1m0thyj commented 7 months ago

Closing in favor of #2078