The hassle with function module calls – part 2
In the last blog post I wrote about my approach to standard function module wrappers. See The hassle with function module calls
Thanks to your participation, there was a lot of very helpful input about this matter from the community. This lead me to a better understanding of exception classes and I now set up a handy mechanism to transform the function module exceptions into appropriate class-based exceptions. Hoping the fruitful discussion is going on, I’d like to share it!
With or without message?
Most function modules pass system messages when raising classical exception. For example, look at function SX_INTERNET_ADDRESS_TO_NORMAL, that can validate mail addresses:
Others, like KNA1_READ_SINGLE, do not so:
Since I didn’t want to loose information, I wanted a mechanism, that transfers the messages into the exception class, if present. If not, the exception should be raised with a simple TEXTID.
With sy-msgid and sy-msgno
Step one: the static coding
When the FM sets a message together with an exception raised, I include the IF_T100_MESSAGE interface in the exception class and create one textid for each message that could be raised in the function module:
As you certainly guess, the name of the textid is a concatenation of message id and message number. For each textid, I assign the appropriate message:
The parameter attributes are to be created first
Now, the coding in the wrapper could look like this:
The name of the textid is created dynamically using the system message fields. Then, a field symbol for it is created and the exception is raised giving the variable parts of the message. It works. But…..
Do I really want to cut/copy this coding block in each wrapper? Of course not. So here comes the next step:
Step 2: the generic exception raiser method
This reduces the coding in the wrapper to one line:
Without a message
In this case, the error is identified by the exception id of the function module. I create one textid for each exception of each function module covered by the exception class. It looks like this:
The text id name is just the name of the function module, followed by the sy-subrc. The text is set according to the exception of the FM.
I skip the static version of the wrapper coding and go directly to the dynamic one! Here is my raiser method (as above, a static public method):
Unfortunately, we have three input parameters here, the SAP FM, the exception class type and the subrc. In the wrapper, the coding is:
The FM name might be too long that you could add the subrc with the underscore at the text id name. In this case, just shorten the iv_module parameter a bit (and do this also with your text id names!).
Conclusion
I just began working with this. There might be things, I just did not think of. I’m looking forward to the discussion!
Hi Jörg,
Thanks for sharing your thoughts.
I'm afraid that your implementation of exception class zcx_Ca03_bp doesn't make too much sense:
In case you know exactly what messages are thrown (e.g. XS007), you may assign these messages to exception IDs, but shouldn't use them as the exception ID (names) themselves. The exception ID should be readable (something like 'CUSTOMER_NOT_EXIST').
In case your messages are dynamic, you should use a dynamic message interface:
For NW release 7.5 or higher, you can use new ABAP syntax: RAISE EXCEPTION TYPE x MESSAGE ABAP News for Release 7.50 - Converting Messages into Exceptions.
For older releases, you'll have to implement it manually.
Hello Shai,
yes, I understand... so to react to
I could create a textid "error_group_address" and assign the message XS138 to it. Do you mean this? The raiser method then gets a parameter with the text id to use. In this case, the exception that has been thrown by the function module does not get lost.
However, I get in trouble when I want to cover more than one function module with the same exception class. Several FM might have exceptions with identical names. How can I distinguish these? There is not enough space in the text id name to include both FM name and exception id...
Furthermore I loose the streamlined one-parameter call of my raiser class.
Still reflecting a real good approach to this...
Well,
I'm afraid there isn't a magic solution for that.
If you wish to preserve the original exception "type" (e.g. error_group_address), you'll have to create a separate exception class for each one of them, in case of dynamic message, or a separate text id, in case of static message.
Again, from pure OO point-of-view, I don't think that one exception class should cover the whole function group. Every exception class should cover one (logical) topic/subject and not be attached to specific FM/FUGR.
Shai, check out the last couple of comments down at the bottom of the page where Jörg and I have some code we are exchanging (I wish there was a way to link directly to a comment)... there is a way to "magically" convert the old style exceptions into class based exception handling.
Jörg said he was planning to write a new blog post about this too... I guess we will have to wait for the SCN upgrade to complete before he posts it.
Hi Raghu,
I've read all the comments 🙂 .
You are right, your code should handle conversion of dynamic message into exception (as I've suggested myself).
However, it doesn't handle assignment of different messages to different exception classes, which must still be done manually.
i.e. You can, of course, use one exception class for all the messages, but if you wish to preserve the logical meaning/topic of different "classic" exceptions, you'll have to create many exception classes.
I see your point... however, I do have just one generic exception class per package. The reason is a pretty lazy one - so that the programmers handling the dynpro/report programs don't have to figure out which exception they are catching. For warehouse management for example, I have one ZCX_WM exception class.
I guess, thinking through it, I could be using sub-classes such as...
This way, I could still use the generic CATCH zcx_wm, but also use ZCX_WM_BIN where I need more definition.
Sorry for the ramble, I was thinking while typing - dangerous, I know - you might have convinced me to go more granular in my exception classes.
The second option sounds fine by me, though you should better add another exception class hierarchy level (e.g. ZCX_WM_TO_UPDATE, ZCX_WM_TO_LOCK).
One generic exception class might sound good from laziness point-of-view, but it's bad in any other aspect. The idea behind different exception classes is that you may handle them differently. You cannot achieve it with one generic exception class.
This is exactly what prompted me to use only one exception class. In my little world of working WM RF barcoding devices, I treat 99.5% (yes, I checked about 5 years and 4 client's 🙂 - roughly 50 dynpro programs) of exceptions exactly the same. I display an error message using a custom popup, log the message into a common log table for all RF transactions, stop the current dynpro process and return control to the user with the cursor on the offending field. That is my little use case and mostly a closed eco-system and why I chose to go this way.
For any one else reading this conversation, if you don't know which way to go - follow Shai's suggestion, it is classical for a reason and for the most part will keep your code more readable and manageable in the long run. DO NOT use my use case as an excuse for laziness - I have years of data supporting this tiny little use case.
Got it.
Though, I wonder: Have you never run into an exception which you really wish to handle (e.g. In case that material isn't maintained in storage location, you want to automatically maintain/create it in storage location) or intentionally ignore (e.g. Plant doesn't exist 😯 raise abort message)?
Oh, yes I have... but it has been about 5 occurrences in about 1000 exceptions raised. I have gone 50-50 in these cases of either putting an IF statement up front after a function call and raising different exceptions - or raising the same exception and checking the TEXTID on the exception class return to do something different - which is an IF statement within the CATCH block.
A slightly more common occurrence which is not clean to handle any way you look at it, is capturing an SAP message and changing it to make more sense or a directional message to the end user. For example, I might change a "Material not in plant xxxx" message to "Please ask your supervisor to extend the material master for material xxxx to plant xxxx".
The user in this case is physically holding the material and saying "$%^ yeah, the material is right here in the plant". Makes perfect sense to us, but the warehouse floor sometimes take these messages too literally and throw tantrums about them!
Shai, you will love this - now that I just stated I have only needed different exceptions in the same program for branching only 5 times in the past... I just started work on a new program (not my usual RF program though, in my defense 😉 ) I have needed about 5 different exception types and I am not even half way through the program!
Hello Jörg,
thank you very much for sharing your ideas.
What I do not understand at the moment.
In the part with messages, why do you create these texts in the class?
Behind the sy-msgid and sy-msgno there are the texts in the message classes, partly even already translated by SAP.
So why should you create these texts?
Thanks again for your blog posts!
Best regards,
Dominik
Hello Dominic
I did not type in any texts. The texts you see in the third screen shot were generated automatically when I assigned the message number to the text id. So I created the text id (XS007, XS138) and then assigned the related message id to them - that's all.
best regards
Jörg
Hi Jörg,
oh, now I understand. Sorry.
I never used se24 to handle exception classes, I always used ADT and there it looks totally different.
But anyway, wouldn't it be better to add two more input parameters to the constructor of the exception class(msgid and msgno).
In the implementation of the constructor you could then use them to fill the t100key.
Best regards,
Dominik
Hello Dominik,
yes, I could, but I was looking for the streamline solution (i.e. only one parameter for the call of the raiser method). Since msgid and msgno are already in sy-msgid and sy-msgno, why create parameters? The raiser class detects the textid to pass from those two syst vars and in the class, the t100key is filled automatically because the textid has the message already assigned.
However - look at the other discussion thread with Shai... there is still brain work to do!
All the best
Jörg
Hi Jörg,
what I thougt of, is as raiser method without even one parameter, because I do not understand what you are doing with it.
Based on the class at the end of my comment the raise_with_message method could have only the raise statement.
raise exception type zcx_gen_exc
exporting
i_msgid = sy-msgid
i_msgno = sy-msgno
i_msgv1 = sy-msgv1
i_msgv2 = sy-msgv2
i_msgv3 = sy-msgv3
i_msgv4 = sy-msgv4.
But I did not test it yet.
Best regards,
Dominik
CLASS zcx_gen_exc DEFINITION
PUBLIC
INHERITING FROM cx_static_check
FINAL
CREATE PUBLIC .
PUBLIC SECTION.
data:
msgv1 type string,
msgv2 type string,
msgv3 type string,
msgv4 type string.
INTERFACES if_t100_message .
METHODS constructor
IMPORTING
!textid LIKE if_t100_message=>t100key OPTIONAL
!previous LIKE previous OPTIONAL
!i_msgid type symsgid
!i_msgno type symsgno
!i_msgv1 type symsgv OPTIONAL
!i_msgv2 type symsgv OPTIONAL
!i_msgv3 type symsgv OPTIONAL
!i_msgv4 type symsgv OPTIONAL.
PROTECTED SECTION.
PRIVATE SECTION.
ENDCLASS.
CLASS zcx_gen_exc IMPLEMENTATION.
METHOD constructor.
data:
key like if_t100_message~t100key.
CALL METHOD super->constructor
EXPORTING
previous = previous.
IF i_msgv1 IS SUPPLIED.
me->msgv1 = i_msgv1.
ELSE.
CLEAR me->msgv1.
ENDIF.
IF i_msgv2 IS SUPPLIED.
me->msgv2 = i_msgv2.
ELSE.
CLEAR me->msgv2.
ENDIF.
IF i_msgv3 IS SUPPLIED.
me->msgv3 = i_msgv3.
ELSE.
CLEAR me->msgv3.
ENDIF.
IF i_msgv4 IS SUPPLIED.
me->msgv4 = i_msgv4.
ELSE.
CLEAR me->msgv4.
ENDIF.
CLEAR me->textid.
IF textid IS INITIAL.
key-msgid = i_msgid.
key-msgno = i_msgno.
key-attr1 = 'MSGV1'.
key-attr2 = 'MSGV2'.
key-attr3 = 'MSGV3'.
key-attr4 = 'MSGV4'.
if_t100_message~t100key = key
ELSE.
if_t100_message~t100key = textid.
ENDIF.
ENDMETHOD.
ENDCLASS.
Hi Jörg,
I now tested my exception class and corrected the comment above.
I think it works, at least for my purposes.
Anyway I am very interested in further discussion about the topic of FM-wrappers.
Best regards,
Dominik
REPORT z_generic_exception_test.
PARAMETERS:
p_adress TYPE sx_address.
DATA:
g_exc TYPE REF TO zcx_gen_exc.
TRY.
CALL FUNCTION 'SX_INTERNET_ADDRESS_TO_NORMAL'
EXPORTING
address_unstruct = p_adress
EXCEPTIONS
error_address_type = 1
error_address = 2
error_group_address = 3
OTHERS = 4.
IF sy-subrc <> 0.
zcl_exc=>raise_with_message( ).
ENDIF.
CATCH zcx_gen_exc INTO g_exc.
MESSAGE g_exc TYPE 'I' DISPLAY LIKE 'E'.
ENDTRY.
*************************************************
CLASS zcl_exc DEFINITION
PUBLIC
FINAL
CREATE PUBLIC .
PUBLIC SECTION.
CLASS-METHODS:
raise_with_message RAISING zcx_gen_exc.
PROTECTED SECTION.
PRIVATE SECTION.
ENDCLASS.
CLASS zcl_exc IMPLEMENTATION.
METHOD raise_with_message.
RAISE EXCEPTION TYPE zcx_gen_exc
EXPORTING
i_msgid = sy-msgid
i_msgno = sy-msgno
i_msgv1 = sy-msgv1
i_msgv2 = sy-msgv2
i_msgv3 = sy-msgv3
i_msgv4 = sy-msgv4.
ENDMETHOD.
ENDCLASS.
Thank you Dominik. I now understand what you mean. So there is no need to create text ids. I never thought of populating the text id in the constructor of the exception class, that's very smart.
However, Shai's response made me think of how I could save also the type of exception and not only the message raised. For instance, SX_INTERNET_ADDRESS_TO_NORMAL raises XS138 for both ERROR_GROUP_ADDRESS and ERROR_ADDRESS exceptions. If I care only for the message, I disregard the exception raised.
I have a question: how do you edit the constructor of the exception class? Since the class has to be public, no editing of the constructor is allowed (says SE24)....
Sorry, I cannot tell you how to solve this in SE24.
Maybe you can find the solution by temporarily switching to "Source Code-Based class builder" / "Quelltextbasierter Class Builder" in settings of SE24.
Then you could copy and paste my source code above and then switch back to normal SE24.
I don't think there is any way with SE24, you get a message saying "Cannot edit the constructor of an exception class" - This may be possible only with ADT.
Another good reason for putting the code in the wrapper instead of the of the Exception class, is that you can control the exception with and without messages and the calling code will follow the OOPs standard better...
zcl_error=>raise_with_message( ).
&
zcl_error=>raise( )
versus...
zcl_error=>raise( in_message_flag = abap_true )
&
zcl_error=>raise( in_message_flag = abab_false )
Jörg,
I have been using almost exactly the same mechanism of handling for a while now.. I have a few additions that I thought I would share.
1) I have an optional BAPIRET structure in the RAISE_xyz methods. This way, I have a consistent way to handle messages even after a BAPI call.
2) Similar to Dominik Di Lorenzo's suggestion, if no parameters are passed to the RAISE_xyz Method, I just use the current SY-MSG** values
3) In the same RAISE_xyz Method, I have a call to a logging mechanism, so that I can quickly turn on logging for a user with a PID to see what error messages they are really getting.
4) While calling the method, I actually reference the textid in the exception class. This way, if I want to know where an error is coming from, I can go to the exception class and do a where used. For example ZCL_CA03_EXC=>RAISE_WITH_ERROR( lv_textid = zcl_ca03_exc=>not_found ). or =>KNA1_READ_SINGLE_1 in your example.
This way, I can link the errors to a message class and still have everything traceable with a where-used. I have used a single ZCX_ exception class and message class for an entire Package and still kept on-going maintenance very easy.
Thanks for sharing!
Raghu
Thank you Raghu - i like the bapiret and logger suggestions.
Jörg, In responding to Dominik, I better understood what you were trying to do with the system messages and not maintaining your own messages.
That said, have you tried Horst Keller's suggestions in this article - ABAP News for Release 7.50 - Converting Messages into Exceptions?
Even in the same class that I use to manage my custom messages, I call it using Keller's suggestion for standard SAP messages that are returned. My code was a slight bit different because of typing. You can also do this for Bapiret. This code can either be in a helper method, or works equally well in a macro.
IF sy-subrc = 4.
RAISE EXCEPTION TYPE zcx_errors
EXPORTING
textid = VALUE scx_t100key( msgid = sy-msgid
msgno = sy-msgno
attr1 = 'TEXT1'
attr2 = 'TEXT2'
attr3 = 'TEXT3'
attr4 = 'TEXT4' )
text1 = CONV sylisel( sy-msgv1 )
text2 = CONV sylisel( sy-msgv2 )
text3 = CONV sylisel( sy-msgv3 )
text4 = CONV sylisel( sy-msgv4 ).
ENDIF.
IF l_bapiret-type = 'E'.
RAISE EXCEPTION TYPE zcx_errors
EXPORTING
textid = VALUE scx_t100key( msgid = l_bapiret-id
msgno = l_bapiret-number
attr1 = 'TEXT1'
attr2 = 'TEXT2'
attr3 = 'TEXT3'
attr4 = 'TEXT4' )
text1 = CONV sylisel( l_bapiret-message_v1 )
text2 = CONV sylisel( l_bapiret-message_v2 )
text3 = CONV sylisel( l_bapiret-message_v3 )
text4 = CONV sylisel( l_bapiret-message_v4 ).
ENDIF.
After the discussions here, I tried a new approach on my own - it looks like this:
That's almost exactly what you wrote - isn't it funny.
I'm planning a blog post about this matter soon.
But regarding the bapiret solution: mostly there is a table of messages from a bapi call. Using this method, I am able to only pass one of the messages. To have a real solution here, the exception should somehow keep the whole table in its memory...
The table for bapiret will not work (I think) because the moment the first error is raised, control drops to the nearest TRY...CATCH. You wouldn't be then able to raise other message. Display too, does the user really need to see all the messages or just one of the type E messages that is returned? I know that is not ideal for a programmer to get only one return, but for an end-user?