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

Error correlator and troubleshoot webview #3243

Merged
merged 75 commits into from
Nov 6, 2024
Merged

Conversation

traeok
Copy link
Member

@traeok traeok commented Oct 15, 2024

Proposed changes

  • Implements an error correlation facility, allowing us and extenders to provide user-friendly summaries of encountered errors
    • We (as well as extenders) can also provide error-specific tips and external resources alongside the correlations to help guide users 😋
    • Summaries are displayed in the initial notification if one was found; otherwise, the full error message is rendered in the dialog with some options
  • Implements a troubleshoot webview, giving users additional information on the error
    • Error code, description, full error details
    • "Copy details" button to copy full error details & stack trace to clipboard
    • Tips (if available for the error correlation)
    • Resources (if available)

If a correlation is found:

  • The error message summary is shown in the dialog. A "More info" option is presented.
  • If "More info" is clicked, a secondary dialog is shown with the full error details and the "Show log" & "Troubleshoot" options mentioned below.

image

After clicking more info:
image

If a correlation is not found:

  • The full error details are shown in the dialog. It also offers a "Show log" and "Troubleshoot" option for diagnosing the error.

image

If "Troubleshoot" is clicked in the dialogs:

  • A troubleshoot webview is opened, displaying the error code, description and full error details - as well as tips and resources (if available)
  • A "Copy details" button is available to copy the contents of the error message (details and stack trace)
Click to reveal troubleshoot screenshot

Release Notes

Milestone: 3.1.0

Changelog:

ZE:

  • Implemented more user-friendly error messages for API or network errors within Zowe Explorer. #3243
  • Use the "Troubleshoot" option for certain errors to obtain additional context, tips, and resources for how to resolve the errors. #3243

ZE API:

  • Leverage the new error correlation facility to provide user-friendly summaries of API and network errors. Extenders can also contribute to the correlator to provide human-readable translations of error messages, as well as tips and additional resources for how to resolve the error. #3243

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds or improves functionality)
  • Breaking change (a change that would cause existing functionality to not work as expected)
  • Documentation (Markdown, README updates)
  • Other (please specify above in "Proposed changes" section)

Checklist

General

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • All PR dependencies have been merged and published (if applicable)
  • A GIF or screenshot is included in the PR for visual changes
  • The pre-publish command has been executed:
    • v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
    • v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

  • There is coverage for the code that I have added
  • I have added new test cases and they are passing
  • I have manually tested the changes

Deployment

traeok and others added 3 commits October 16, 2024 14:02
…3248)

* fix(api): Fix profile references being lost when cache is refreshed

Signed-off-by: Timothy Johnson <[email protected]>

* fix: Pass profile instead of profile name for updating creds

Signed-off-by: Trae Yelovich <[email protected]>

---------

Signed-off-by: Timothy Johnson <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Co-authored-by: Trae Yelovich <[email protected]>
This reverts commit 53e9518.

Signed-off-by: Trae Yelovich <[email protected]>
Copy link

codecov bot commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 91.64345% with 30 lines in your changes missing coverage. Please review.

Project coverage is 92.99%. Comparing base (085e71d) to head (df1534c).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ges/zowe-explorer-api/src/utils/ErrorCorrelator.ts 90.66% 7 Missing ⚠️
...kages/zowe-explorer/src/trees/uss/UssFSProvider.ts 89.13% 5 Missing ⚠️
packages/zowe-explorer-api/src/fs/BaseProvider.ts 78.57% 3 Missing ⚠️
...kages/zowe-explorer/src/trees/shared/SharedInit.ts 50.00% 3 Missing ⚠️
packages/zowe-explorer/src/utils/AuthUtils.ts 92.68% 3 Missing ⚠️
...kages/zowe-explorer/src/trees/job/JobFSProvider.ts 93.93% 2 Missing ⚠️
...kages/zowe-explorer/src/utils/TroubleshootError.ts 92.00% 2 Missing ⚠️
...s/zowe-explorer/src/commands/UnixCommandHandler.ts 66.66% 1 Missing ⚠️
.../zowe-explorer/src/trees/dataset/DatasetActions.ts 92.30% 1 Missing ⚠️
...ges/zowe-explorer/src/trees/dataset/DatasetTree.ts 50.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3243      +/-   ##
==========================================
- Coverage   93.09%   92.99%   -0.10%     
==========================================
  Files         114      116       +2     
  Lines       11834    12008     +174     
  Branches     2572     2637      +65     
==========================================
+ Hits        11017    11167     +150     
- Misses        815      839      +24     
  Partials        2        2              

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

zFernand0
zFernand0 previously approved these changes Nov 5, 2024
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

Changes LGTM! 😋
left a small question/suggestion, but feel free to ignore if it doesn't make much sense 😋

@@ -83,6 +83,7 @@ export class AuthUtils {
const correlation = ErrorCorrelator.getInstance().correlateError(moreInfo?.apiType ?? ZoweExplorerApiType.All, errorDetails, {
profileType: profile?.type,
...Object.keys(moreInfo).reduce((all, k) => (typeof moreInfo[k] === "string" ? { ...all, [k]: moreInfo[k] } : all), {}),
templateArgs: { ...(moreInfo.templateArgs ?? {}), profileName: profile?.name ?? "" },
Copy link
Member

Choose a reason for hiding this comment

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

Hey, dumb question...
if a profile name is provided in the templateArgs and profile == null, aren't we overwriting the profile name with ""
image

Also, we should be able to spread the templateArgs without the need for a ?? {} check 😋

Suggested change
templateArgs: { ...(moreInfo.templateArgs ?? {}), profileName: profile?.name ?? "" },
templateArgs: profile ? { ...moreInfo.templateArgs, profileName: profile.name } : { ...moreInfo.templateArgs },

Copy link
Member Author

@traeok traeok Nov 5, 2024

Choose a reason for hiding this comment

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

If you have access to the profile at the time of the errorHandling call, it should be passed into moreInfo as the profile property. The template args are essentially key/value pairs of strings and are passed right into Handlebars. Your suggestion highlighted that changing the spread to come after profileName would prevent this overwrite. I'll introduce that change.

I think it would be cleaner and less error-prone to use optional chaining to spread the template args, so I'll update that.

t1m0thyj
t1m0thyj previously approved these changes Nov 5, 2024
Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @traeok! Left a few questions

JillieBeanSim
JillieBeanSim previously approved these changes Nov 5, 2024
Signed-off-by: Billie Simmons <[email protected]>
JillieBeanSim
JillieBeanSim previously approved these changes Nov 5, 2024
@traeok
Copy link
Member Author

traeok commented Nov 5, 2024

Hi all, sorry for the confusion but unfortunately the error in #3289 will not be resolved with this work.
Currently there are bugs with promptCredentials that is causing the profile to not propagate to the file system.

Once this is resolved, the "Try again" button in the editor will work as it will use the updated creds. We should not prompt here as extenders will use readFile to access data set and USS resources, so this would be a breaking change for extenders if we halted a readFile operation to prompt the user for new creds.

I can address the prompt bugs in a separate PR since this one already has a bunch of commits/feedback.

t1m0thyj
t1m0thyj previously approved these changes Nov 5, 2024
Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @traeok!

zFernand0
zFernand0 previously approved these changes Nov 5, 2024
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

LGTM!

@traeok traeok dismissed stale reviews from zFernand0 and t1m0thyj via df1534c November 6, 2024 15:49
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

still LGTM! 😋

@zFernand0 zFernand0 merged commit e07e839 into main Nov 6, 2024
33 of 34 checks passed
@zFernand0 zFernand0 deleted the fix/error-handling-saves branch November 6, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

5 participants