Try catch block I usually use in COS

Hi!

Want to share with you code snippet of try catch block I usually use in methods which should return %Status. 


{ 
 try {
  	$$$TOE(sc,StatusMethod())
 }
 catch e {
 	set sc=e.AsStatus()
 	do e.Log()
 }

Quit sc 
}

Here $$$TOE is a short form of $$$TROWONERROR macro.

Inside macro StatusMethod is any method you call which will return %Status value. This value will be placed into sc variable.

In case of sc contains error execution will be routed to try catch block. You can wrap any Status methods calls in your code if you need to catch the errors coming from them.

In try catch block I place my logic and have to mandatory calls:

s sc=e.AsStatus() to get the status of error.

D e.Log()  - to place all the stack of error to standard Application Error log which you can find in Management portal on this page:

http://localhost:57772/csp/sys/op/UtilSysAppErrorNamespaces.csp?$NAMESPACE=SAMPLES&Recent=1

How do you handle errors in your COS logic?

  • + 2
  • 0
  • 1306
  • 21

Comments

I often do this too, I think this sort of pattern is a good way to make the error handling simple and consistent.

I'm not really a fan of the status codes. They seem like a legacy item for the days prior to try/catches when traps were used. 

Having to maintain code to handle possible exceptions AND "errors" means more control flow to check and handle problems through the various levels of code.

Say that code above is within a method that's within another method that's called from the main part of the program. Now, you need three levels of try/catches. That's a lot of extra lines of code (code smell), lost errors and performance degradation.

Instead, I encourage my developers to use the fail fast methodgy. We use try/catches as high as possible in the code or when absolutely necessary, like when something is prone to exceptions but shouldn't stop the overall program from running. 

I agree with you. As mentioned it is an option if you need to use %Status as a method result. 

And what about e.Log() - do you use it to log errors or use something else? 

We use various methods and macros to log exceptions, it just depends on the situation. 

Status codes still have a place along side of Try/Catch in my opinion.  They really only serve to indicate the ending state of the method called.  This is not necessarily an error.  I agree that throwing an exception for truly fatal errors is the best and most efficient error handling method.  The issues is what does "truly fatal" mean?  There can be a lot of grey area to contend with.  There are methods where the calling program needs to determine the correct response.  For example take a method that calculates commission on a sale.   Clearly this is a serious problem on a Sales order.  However, it is less of an issue on a quotation.  In the case of the latter the process may simply desire to return an empty commissions structure.

Placement of try/catch blocks is a separate conversation.  Personally I find using try/catch blocks for error  handling to be clean and efficient.  The programs  are easier to read and any recovery can be consolidated in one place, either in or right after the catch.  I have found that any performance cost is unnoticeable in a typical transactional process.  It surely beats adding IF statements to handle to handle the flow.  For readability and maintainability I also dislike QUITing from a method or program in multiple places.  

So where is the "right" place for a try/catch?  If I had to pick one general rule I would say you should put the try/catch in anyplace where a meaningful recovery from the error/exception can be done and as close to the point where the error occurred as possible.   I the above example of a Commission calculation method I would not put a try/catch in the method itself since the method can not perform any real recovery.  However I would put one in the Sales order and Quotation code.

There are many methods to manage program flow under error/exception situations;  Try/Catch,  Quit and Continue in loops are a couple off the top of my head.  Used appropriately they can create code that is robust, readable and maintainable with little cost in performance.    

I see where you are coming from with having a status code returned to notify a problem exists. 

Personally, I'd calculate comissions in a seperate single responsibility class that extends an abstract class for interface segregation. That class implements three public methods: Calculate(), HasError(), GetError()

//calculate then commission: 

Set tCommission = tUsedCarCommission.Calculate()

If tUsedCarCommission.HasError() 

      //log the error or do something with it 

 

It's very similar to what you'd do with traditional status types but without having to deal with passing in references. Also, IMO, it's very clear what's going on if the commission has an error. 

I agree with both Nic and Rich.

The issue with e.Log() is that it clutters up the error log with repetitive entries, each subsequent one with less detail than the prior. The same thing happens in Ensemble when errors bubble through the host jobs.

The trick here is knowing when to log an error verses when to just bubble it up. Using Nic's method we lose the context of the stack since the log isn't written until the entry method with the Try/Catch. Using your method we get noise in the logs, but at least the first one has the detail we'll need.

I believe the root problem here is re-throwing the status. An exception should represent something fatal and usually out of the applications control (e.g. FILEFULL) while a %Status is a call success indicator. To that end your code could be refactored to just Quit on an error instead of throwing it. That way a single log is generated in the method that triggered the exception and the stack is accurate.

However this doesn't work well in nested loops. In that case a Return would work unless there is a cleanup chore (e.g. releasing locks, closing transactions, etc). I haven't come up with a pattern for nested loops that doesn't clutter up the source with a bunch of extra $$$ISERR checks that are easily missed and lead to subtle bugs.

Personally I use your style without logging because:

  • Every method uses the same control structure
  • Works with nested loops without extra thought
  • Can ignore errors by simply invoking with Do instead of $$$TOE/$$$ThrowOnError
  • Cleanup code is easy to find or insert
  • Using ByRef/Output parameters makes it trivial to refactor to return more than one value

I do lose the ability to see an accurate stack trace but most of the time the line reference in the error is enough for me to quickly debug an issue so it is an acceptable trade-off. Only in the most trivial methods is the Try/Catch skipped.

All that said Nic's style is a solid approach too. By removing a bunch of boilerplate Try/Catch code it lets the programmer focus on the logic and makes the codebase much easier on the eyes.

OK. But how do you call methods which return %Status?

Do you raise Error? Or you check Error status with if? Or do you ignore status at all?

Most of my methods look like this:

Method MyMethod() As %Status {
  // Initialization - note locking and transactions only when needed
  Set sc = $$$OK
  Lock +^SomeGlobal:5 If '$TEST Quit $$$ERROR($$$GeneralError,"Failed to obtain lock")

  Try {
    TSTART

    // When error is significant
    $$$ThrowOnError(..SomeMethod())

    // When error can be ignored
    Do ..SomeMethod()

    // When only certain errors apply
    Set sc = ..SomeMethod()
    If $$$ISERR(sc),$SYSTEM.Status.GetErrorText(sc)["Something" $$$ThrowStatus(sc)
  
    TCOMMIT
  }
  Catch e {
    TROLLBACK
    Set sc = e.AsStatus()
  }
  Lock -^SomeGlobal
  Quit sc
}

 

>$SYSTEM.Status.GetErrorText(sc)["Something"

Would not always work correctly in applications with users requesting content in several languages (for example web app). Why not use error codes?

If $SYSTEM.Status.GetErrorCodes(sc)[$$$GeneralError $$$ThrowStatus(sc)

Agreed - GetErrorCodes() is the right thing to do from an I18N perspective.

I prefer doing this more correctly using the right API for matching status values:

If $system.Status.Equals(sc,$$$ERRORCODE($$$GeneralError),$$$ERRORCODE($$$MoreSpecificStatusCode),...) {
  // Handle specific error(s)
}

The use of ' can result in unexpected behaviour when you are checking for a 4 digit code, but are handling a 3 digit code...

Note that it's also safer to wrap status codes in $$$ERRORCODE($$$TheErrorCode), but that may not be necessary depending on the specific context.

What's the advantage of using $$$ERRORCODE macro? ^%qCacheObjectErrors global contains the same values.

For more advanced error analysis, such as conversion of error %Status-es into user-friendly messages (as I described in another comment), $System.Status.DecomposeStatus will provide the parameters of the error message as well. These are substituted in to the localizable string.

For example, here's a foreign key violation message from %DeleteId on a system running in Spanish:

INSYNC>Set tSC = ##class(Icon.DB.CT.TipoDocumento).%DeleteId(50)                 
INSYNC>k tErrorInfo d $System.Status.DecomposeStatus(tSC,.tErrorInfo) zw tErrorInfo
tErrorInfo=1
tErrorInfo(1)="ERROR #5831: Error de Foreign Key Constraint (Icon.DB.CC.AllowedGuaranteeTypes) sobre DELETE de objeto en Icon.DB.CT.TipoDocumento: Al menos existe 1 objeto con referencia a la clave CTTIPODOCUMENTOPK"
tErrorInfo(1,"caller")="zFKTipoDocDelete+4^Icon.DB.CC.AllowedGuaranteeTypes.1"
tErrorInfo(1,"code")=5831
tErrorInfo(1,"dcode")=5831
tErrorInfo(1,"domain")="%ObjectErrors"
tErrorInfo(1,"namespace")="INSYNC"
tErrorInfo(1,"param")=4
tErrorInfo(1,"param",1)="Icon.DB.CC.AllowedGuaranteeTypes"
tErrorInfo(1,"param",2)="Icon.DB.CT.TipoDocumento"
tErrorInfo(1,"param",3)="DELETE"
tErrorInfo(1,"param",4)="CTTIPODOCUMENTOPK"
tErrorInfo(1,"stack")=...

The "param" array allows clean programmatic access to the details of the foreign key violation, independent of language.

Of course, these level of detail in these error messages may be subject to change across Caché versions, so this is a *great* thing to cover with unit tests if your application relies on it.

I will leave the logging issue alone as I don't see it as being the main point of the example.  It could also be a thread by itself.

The issue of using a bunch of $$$ISERR or other error condition checks is exactly why I like using throw with try/catch.  I disagree that it should only be for errors outside of the application's control.  However it is true that most of the time you are dealing with a fatal error.  Fatal that is to the current unit of work being performed, not necessarily to the entire process.

I will often use code like

set RetStatus = MyObj.Method() 

throw:$$$ISERR(RetStatus) ##class(%Exception.StatusException).CreateFromStatus(RetStatus)

The post conditional on the throw can take many forms, this is just one example.

Where I put the Try/Catch depends on many factors such as:

  • Where do I want recovery to happen?
  • use of the method and my code 
  • readability and maintainability of the code
  • ...

I the case of nested loops mentioned I think this is a great way to abort the process and return to a point, whether in this program or one farther up the stack, where the process can be cleanly recovered or aborted.

Well, I think a major question is: What do you use to return runtime information to your caller when you implement your own code? Do you return a %Status object, or something similar, or do you throw exceptions and don't return anything.

Most code snippets I have seen here make use of try/catch, but do return a status code itself. 

Personally, I prefer to use try/catch blocks and throw errors when I encounter issues at runtime. The try/catch philosophy is optimized for the case that everything goes well and exceptions are the, well, exception. Handling a status object is not as clean from a code maintenance perspective (more lines of code within your logic), but allows you to handle multiple different scenarios at once (it was okay/not okay, and this happened...)

Obviously, this is my personal preference.

What do you do when you need to cleanup things like locks?  Put the unlock code both in the Catch and before the Quit?

Well, that depends on where you did take the lock.

In your previous example you take a lock right before the try block, so you can release it directly after the try/catch block.

If you take a lock in your try block, you have to put the unlock code both in the catch block and at the end of the try block. I would not place the unlock code outside of the try/catch block. This is a case where a try/catch/finally construct would definitely help, as you could place the unlock code in the finally block. 

Other than locks, there are a few other cases where cleanup may be needed whether or not something goes wrong:

  • Closing SQL cursors that have been opened
  • Ensuring that the right IO device is in use and/or returning to the previous IO redirection state.

There are probably more of these too.

Here's the convention we use for error handling, logging, and reporting in InSync (a large Caché-based application):

  • We have TSTART/TCOMMIT/TROLLBACK in a try/catch block at the highest level (typically a ClassMethod in a CSP/Zen page). There isn't much business logic in here; it'll call a method in a different package.
  • If anything goes wrong in the business logic, an exception is thrown. The classes with the business logic don't have their own try/catch blocks unless it's needed to close SQL cursors, etc. in event of an exception. After the cleanup is done, the exception is re-thrown. (Unfortunately, this means that cleanup code may be duplicated between the try and catch blocks, but there's typically not too much duplication.) The classes with business logic also don't have their own TSTART/TCOMMIT/TROLLBACK commands, unless the business logic is a batch process in which parts of the process may fail and be corrected later without impacting the whole thing; such a case may also call for a nested try/catch to do the TROLLBACK if something goes wrong in part of the batch. In this case the error is recorded rather than re-throwing the exception.
  • We have our own type of exception (extending %Exception.AbstractException), and macros to create exceptions of this type from:
    • Error %Status codes
    • Error SQLCODEs and messages
      • SQLCODE = 100 can be treated as an error, "alert", or nothing.
    • Other types of exceptions
  • Exceptions of our custom type can also be created to represent a general application error not related to one of those things, either a fatal error, or something the user can/should fix - e.g., invalid data or missing configuration.
  • The macros for throwing these exceptions also allow the developer to provide a localizable user-friendly message to explain what went wrong.
  • When an exception is caught in the top level try/catch (or perhaps in a nested try/catch in a batch process), we have a macro that logs the exception and turns it into a user-friendly error message. This might just be a general message, like "An internal error occurred (log ID _______)" - the user should never see <UNDEFINED>, SQLCODE -124: DETAILS ABOUT SOME TABLE, etc.
  • Our persistent classes may include an XDATA block with localizable error messages corresponding foreign and unique keys in the class and types of violations of those keys. For %Status codes and SQLCODEs corresponding to foreign/unique key violations, the user-friendly error message is determined based on this metadata.
  • Logging for these exceptions is configurable; for example, exceptions representing something the user can/should fix are not logged by default, because they're not an error in the application itself. Also, the log level is configurable - it might be all the gory detail from LOG^%ETN, or just the stack trace. Typically, verbose logging would only be enabled system-wide briefly for specific debugging tasks. For SQL errors, the SQL statement itself is logged if possible.

I thought this convention was too complicated when I first started working with it, but have come to see that it is very elegant. One possible downside is that it relies on a convention that any method in a particular package (InSyncCode, in our case) might throw an exception - if that isn't respected in the calling code, there's risk of a <THROW> error.

I mentioned the InSync approach previously on https://community.intersystems.com/post/message-error-csppage . Unfortunately, it's coupled with several parts of the application, so it'd be quite a bit of work to extract and publish the generally-applicable parts. I'd like to do that at some point though.

Timothy! Thanks for sharing this!

It' can be a standalone post as "Error and resource handling in large Caché ObjectScript project".

Thank you!

 

A rather subtle point that I haven't seen discussed here is actually about how TSTART/TCOMMIT/TROLLBACK should be handled when triggering a TROLLBACK from code that may be part of a nested transaction. Given that a lot of the code I write may be called from various contexts and those calling contexts may already have open transactions, my preferred transaction pattern is as follows:

Method SaveSomething() As %Status {
  Set tStatus = $$$OK
  Set tInitTLevel = $TLevel
  Try {
    // Validate input before opening transaction so you can exit before incurring any major overhead
    TSTART
    // Do thing 1
    // Do thing 2
    // Check status or throw exception
    TCOMMIT
  }
  // Handle exception locally due to local cleanup needs
  Catch ex {
    Set tStatus = ex.AsStatus()
  }
  While ($TLevel > tInitTLevel) {
    // Only roll back one transaction level as you make it possible for the caller to decide whether the whole transaction should be rolled back
    TRollback 1
  }
  Quit tStatus
}

(Edited for formatting.)