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

standardizing error handling #2062

Closed
wants to merge 16 commits into from
Closed

Conversation

ATorrise
Copy link
Contributor

@ATorrise ATorrise commented Feb 22, 2024

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

@ATorrise ATorrise linked an issue Feb 22, 2024 that may be closed by this pull request
Copy link

codecov bot commented Feb 22, 2024

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 ⚠️
Additional details and impacted files
@@            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              

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

@ATorrise ATorrise marked this pull request as ready for review February 27, 2024 21:06
Signed-off-by: Amber Torrise <[email protected]>
Copy link
Member

@gejohnston gejohnston left a comment

Choose a reason for hiding this comment

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

I suggested a modification in the changelog.

packages/cli/CHANGELOG.md Show resolved Hide resolved
@gejohnston
Copy link
Member

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
Copy link
Member

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?

Copy link

sonarcloud bot commented Feb 27, 2024

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarCloud

@adam-wolfe
Copy link
Contributor

adam-wolfe commented Feb 28, 2024

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
Copy link
Member

The previous related issue is #935
The previous related PR is #1738

@t1m0thyj
Copy link
Member

t1m0thyj commented Mar 6, 2024

Closing in favor of #2078

@t1m0thyj t1m0thyj closed this Mar 6, 2024
@ATorrise ATorrise deleted the file-not-found-error-messages branch March 26, 2024 15:02
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.

File Not Found Messaging Inconsistent
4 participants