zowe / zowe-explorer-vscode

Visual Studio Code Extension for Zowe, which lets users interact with z/OS Data Sets, Unix System Services, and Jobs on a remote mainframe instance. Powered by Zowe SDKs.
Eclipse Public License 2.0
162 stars 90 forks source link

Centralized error handling #388

Closed jellypuno closed 2 years ago

jellypuno commented 4 years ago

It would be nice to have a new function that handles error messages.

For Example:

If (err.errorCode === 401) {
    await ErrorHandling("ZWEEX401E")
}
public async ErrorHandling(errCode) {
    switch(errCode) {
        case "ZWEEX401E": {
             vscode.window.showErrorMessage(localize("getChildren.error.response", "Invalid 
             Credentials for profile") + `\n${this.label}\n` + ". Please make sure that the 
             credentials are valid.");

And then we could have a document like the api-layer troubleshooting for easier troubleshooting.

zFernand0 commented 4 years ago

This will definitely be helpful if the decision is to move forward with Message IDs. If someone has any concerns about this, please feel free to bring it up.

This centralized location could also be used for handling any error mapping as well (if we decide to go that route too).

My only concerns are:

jellypuno commented 4 years ago

I created a POC for this one. The message ID format is:

ZE = Zowe Explorer ZN = Zowe Node (The class where the error is triggerred) 401 = https Status Code S = Severe Error

Capture

I think this is very helpful when we are debugging.

zFernand0 commented 4 years ago

I like where this is going! 👍

Questions:

  1. What happens if the Class name is too long or has more than two distinct words? (e.g. ZoweAwesomeNode)
  2. Since we already have the HTTP Status Code, do we need the severity level?
  3. Can there be Different types of message with different "severity" levels? (i.e. Trace, Debug, Info, Warning, Failure)
jellypuno commented 4 years ago

I like where this is going! 👍

Questions:

  1. What happens if the Class name is too long or has more than two distinct words? (e.g. ZoweAwesomeNode)
  2. Since we already have the HTTP Status Code, do we need the severity level?
  3. Can there be Different types of message with different "severity" levels? (i.e. Trace, Debug, Info, Warning, Failure)

@zFernand0 Good questions! I think we need to discuss this. Severity Levels are just patterned from Mainframe Message IDs. We can define it however we want. I think API-ML has implemented this. I just don't know where it is though. We can use that as a reference.

What happens if the Class name is too long or has more than two distinct words? (e.g. ZoweAwesomeNode)

Basically the goal is to provide a Code for the Class. So far the message ID is just 8. We can make it 10 or even 30. It will be logged in zowe logs anyway.

zFernand0 commented 4 years ago

We can use that as a reference.

Absolutely! @petr-galik do you know where can we find information about the standard message IDs implemented by the APIML?

Basically the goal is to provide a Code for the Class. So far the message ID is just 8. We can make it 10 or even 30. It will be logged in zowe logs anyway.

I agree that it can contain lots of characters as long as we don't display them in the descriptive error messages that we have already. (e.g. Unable to find file: HLQ.MYDSNAME(MEMBER) was probably already deleted.)

jellypuno commented 3 years ago

I think extenders should have an error message tag

jellypuno commented 3 years ago
katelynienaber commented 3 years ago

@jellypuno @zFernand0 Hey, so I'm doing some research, and suggested for smaller TS projects is to throw exceptions, not to use error codes. With error codes, we'd have to keep the documentation & handling function up-to-date for the error codes all the time.

Also another thought, maybe we shouldn't be throwing exceptions at all? Instead we could return some special data type, so that we're locked into best practices by using this type. I like this approach for instance: https://benjaminjohnson.me/blog/typesafe-errors-in-typescript

Basically, we'd make a new data type that looks like this:

type ResultSuccess<T> = { type: 'success'; value: T }
type ResultError = { type: 'error'; error: Error }
type Result<T> = ResultSuccess | ResultError

And we'd return this data type from our functions...especially from all the API functions. It would be nice to use it everywhere in ZE though.

If we use this, extenders (and us) can't ignore errors that are returned. We can't accidentally miss catching an error and thus crashing the app.

katelynienaber commented 3 years ago

Also linking this to here: https://github.com/zowe/vscode-extension-for-zowe/issues/388

lauren-li commented 3 years ago

Should #1292 be linked to this? (V1 Conformance - Create a best practice documentation for error handling)

katelynienaber commented 3 years ago

@lauren-li Yes I already linked it above...

lauren-li commented 3 years ago

@katelynienaber Ah, I see it linked in @jellypuno's comment now. Apologies for the duplicate link!

katelynienaber commented 3 years ago

THis is a bit related to: https://github.com/zowe/vscode-extension-for-zowe/issues/1346

jellypuno commented 3 years ago

Tasks:

  1. Research on how to use Imperative Handler

    API that extenders could use VSCode API for logging and dialogs

  2. Imperative Error Handler accessible to extenders
  3. Change ZE to use this imperative handler. Check all error handling in the code?
  4. Research if we could use Logging - Output View?

    Research log4js appenders to write log file and output view

jellypuno commented 3 years ago

Do we have breaking changes that will affect extenders? We need to research on this one.

jellypuno commented 2 years ago

Reference #1518

jellypuno commented 2 years ago

Remaining Tasks is to implement the Error Logging API in ZE codebase