AXForum  
Вернуться   AXForum > Блоги > Logger
NAV
Забыли пароль?
Зарегистрироваться Правила Справка Пользователи Сообщения за день Поиск Все разделы прочитаны

Оценить эту запись

X++, the catch Michael Fruergaard Pontoppidan November 24, 2016

Запись от Logger размещена 03.06.2019 в 09:42
Обновил(-а) Logger 10.06.2019 в 21:23

X++, the catch
mfp Michael Fruergaard Pontoppidan November 24, 2016

"When building a cannon, make sure the cannon ball comes out in the right direction." This is a piece of advice I heard many years ago. I think, we in generally have been following the advice in the Dynamics AX platform group. The APIs and designs have been easy to understand, and without side-effects. This blog post describes an exception – if you are an X++ developer, you better pay attention: The cannon in your hands will not behave as you might expect.

Consider this method:

void post(int _amount)
{
MyPostings debit;
MyPostings credit;

try
{
ttsBegin;

debit = MyPostings::find('debit', true);
debit.Amount += _amount;
debit.update();

credit = MyPostings::find('credit', true);
credit.Amount -= _amount;
credit.update();

ttsCommit;
}
catch
{
error("Something bad happened - try again later");
}
}



Is this code safe from a transactional point-of-view? When reading the code, the intention is clear: Either both the debit and credit accounts are updated, or neither are. One of the first things we learned as X++ developers is that exceptions will roll-back the current transaction, and be caught outside the transaction. We also learned that 2 exception types (UpdateConflict and DuplicateKey) can be caught inside the transaction. Regardless of the type of exception, the code is robust, as we'll never hit the final ttsCommit - so, it appears this code is ok.

Now, this post() method might be so useful and robust, that someone decides to reuse it. For example, like this:

void postList(List _list)
{
ListEnumerator enum = _list.getEnumerator();

ttsBegin;

while (enum.moveNext());
{
post(enum.current());
}

ttsCommit;
}



Are we still transactional reliable? When reading the code, the intention is clear: Either every item in the list is posted, or none are – by using the post() method that ensures the integrity of each posting.

This code is not reliable - it is a disaster waiting to happen!

Here is what can (and will) happen. Suppose we call the postList() with a list containing the integer 17:

postList() starts a new transaction – ttsLevel is now 1
postList() calls post(17)
post() increases the transaction level – ttsLevel is now 2
post() updates the debit record without problems – this is now part of the transaction.
post() attempts to update the credit record. Suppose this raises an updateConflict exception – someone else has updated the row between the select and the update statement.
The updateConflict is caught by the catch statement in the post() method – YES, catch "all" will catch updateConflict and duplicateKey exceptions, even inside a transaction.
ttsLevel is automatically set back to the value it had on the try-statement – ttsLevel is now 1
The error message is written to the infolog
The post() method ends gracefully – but only the update of debit was added to the transaction
postList() call ttsCommit – ttsLevel is now 0. The update of debit was committed without the update of credit.
The reuse of the post() method resulted in a partial transaction being committed!

Here is the lesson:
Never use catch "all" without explicitly catching updateConflict and duplicateKey exceptions.

One option is to stop using the catch "all" – but often that is not possible. When using catch "all", you must explicitly also catch updateConflict and duplicateKey. The catch logic for updateConflict/duplicateKey falls in 4 patterns:

1. Playing-it-safe
catch (Exception::UpdateConflict)
{
if (appl.ttsLevel() != 0)
{
throw Exception::UpdateConflict;
}
if (xSession::currentRetryCount() >= #RetryNum)
{
throw Exception::UpdateConflictNotRecovered;
}
//TODO: Eventual reset of state goes here.
retry;
}

This is the textbook example, and recommended in the Inside Dynamics AX series. This implementation will escalate the exception to the next level when a transaction is still active. This is a reliable implementation – when your code supports retry. This is the most commonly used pattern, and can be found throughout the standard application. Note: The actual implementation in the text book is slightly different, but semantically the same as mine – I just prefer readable code.

2. I'm-feeling-lucky

catch (Exception::UpdateConflict)
{
if (xSession::currentRetryCount() >= #RetryNum)
{
throw Exception::UpdateConflictNotRecovered;
}
//TODO: Eventual reset of state goes here.
retry;
}

The main reason for allowing catching the exception within the transaction is to enable recovery without rolling back everything. This implementation is a bit risky, and should only be used with outmost caution. Only use it when the try block contains just one update statement. The transaction scope will not help you, you need to ensure that entire try block can be repeated – including eventual subscribers to the various events raised. One example where this pattern is used is in InventDim::findOrCreate(). The xSession class has a few helper methods to find the table ID for the table raising the UpdateConflict or DuplicateKey exception – they can be handy in more complicated recovery implementations.

3. Not-my-problem

catch (Exception::UpdateConflict)
{
throw Exception::UpdateConflict;
}

This is the way to prevent the catch "all" block from catching updateConflicts and duplicateKey exceptions. This is also a reliable implementation - use this when you don't support retry.

4. I-need-to-log-this

catch (Exception::UpdateConflict)
{
if (appl.ttsLevel() != 0)
{
throw Exception::UpdateConflict;
}
this.myLogMethod();
}

The myLogMethod() should be called from catch updateConflict, catch duplicateKey and catch "all". Typically, you only want to log something when you assume you are first on the stack; but add the safe-guard anyway, someone may reuse your method and break that assumption. The BatchRun class contains an example of this.

All that said; there is still the risk of additional exception types, that can be caught inside a transaction, introduced in future versions. None of the patterns above accounts for those. The best way of guarding against this would be to throw an exception in the catch "all" block, if the ttsLevel is not 0. It should be obvious that introducing new exceptions of this type is a breaking change.

What does reliable code look like?
Let's rewrite the post() method to be reliable using Playing-it-safe pattern, and guarding against future inside-transaction-catchable exceptions.

void post(int amount)
{
#OCCRetryCount
MyPostings debit;
MyPostings credit;

try
{
ttsBegin;

debit = MyPostings::find('debit', true);
debit.Amount += amount;
debit.update();

credit = MyPostings::find('credit', true);
credit.Amount -= amount;
credit.update();

ttsCommit;
}
catch (Exception::UpdateConflict)
{
if (appl.ttsLevel() != 0)
{
throw Exception::UpdateConflict;
}
if (xSession::currentRetryCount() >= #RetryNum)
{
throw Exception::UpdateConflictNotRecovered;
}
retry;
}
catch (Exception:uplicateKeyException)
{
if (appl.ttsLevel() != 0)
{
throw Exception:uplicateKeyException;
}
if (xSession::currentRetryCount() >= #RetryNum)
{
throw Exception:uplicateKeyExceptionNotRecovered;
}
retry;
}
catch
{
if (appl.ttsLevel() != 0)
{
throw error("Something happened, that the logic was not designed to handle – please log a bug.");
}
error("Something bad happened - try again later");
}
}



What about deadlock exceptions?
Deadlock exceptions cannot be caught inside a transaction – so they essentially behaves like Exception::Error. This means you are not at risk of incomplete transactions, even if they are not caught explicitly.

Typically, the catch of a deadlock simply contains a retry statement – the idea is that once the deadlock transaction is aborted the deadlock situation is lifted, and a retry will succeed. I've seen two customer cases of repeatable deadlocks, where the retry statement immediately causes a new deadlock to occur – the system appears to hang, SQL is wasting precious resources to find deadlocks and throw the exception (several times per second), which just prolongs the run for the main thread. I'd propose writing catching of deadlocks like this:

catch (Exception:eadlock)
{
if (xSession::currentRetryCount() >= #RetryNum)
{
throw Exception:eadlock;
}
//TODO: Eventual reset of state goes here.
retry;
}

Can I catch exception::error instead of catch "all"?
Yes; it would solve the transactional problem – but with a negative side-effect. The downside is that exception::error is just one of many (23 in the latest release) exception types – so you will not be catching the remaining 22. The benefit of cause is that it will not catch updateConflict and duplicateKey exceptions either, so you are avoiding the main problem.

Tags X++
Comments (23)
You must be logged in to post a comment.

Logger
March 30, 2017 at 4:33 am
Fixed in
https://ax.help.dynamics.com/en/wiki...-5-march-2017/

Log in to Reply
Vidir Reyr Bjorgvinsson
December 15, 2016 at 8:44 am
Very nice post :-). Try/Catch and TTS level is very important to handle correctly and should be taught early on.
Might just add, you can have specific catches, like updateConflict and duplicateKey … however, it is good practice to always include the generic one at the end (Just “Catch”). Becomes the “if none of the above”, that last one would be the critical error that might abort the whole scenario and all tts as we have not created a specific error handling for it and do not want to take any chances.

Additionally, I read Martin Dráp comments above, and I must agree, give us more details about the error so we can act accordingly. Should not effect integrity much if catch is otherwise handled generally with critical level of rollback. But more details can help us better recover from exceptions, provide users with more accurate error notifications and assist in debugging and logging.

But Again, very good post, I will be sure to share it with my colleagues.

Log in to Reply
Vladi
December 1, 2016 at 9:11 am
Is it good that with “feeling lucky” or actually retry scenario would be debit.amount always increaesed in case that update conflit comes only with the credit update. It s also dangerous because member variables dont get their “first state”.

Log in to Reply
Velislav M
November 29, 2016 at 9:03 am
Not a fan of compiler magic, but this is an example of how syntax sugar makes us fat and lazy. Can we have some more, please!
Размещено в Без категории
Просмотров 203506 Комментарии 2
Всего комментариев 2

Комментарии

  1. Старый комментарий
    Velislav M
    November 29, 2016 at 9:03 am
    Not a fan of compiler magic, but this is an example of how syntax sugar makes us fat and lazy. Can we have some more, please!
    We already know what the correct “catch-all” block should look like…
    So, why can’t we have a keyword that automatically emits all the boilerplate code for handling correctly the update-conflict and duplicate-key exceptions?

    I mean, going forwrad, it would be nice if we could write code that can be fully covered by unit tests without too much hassle.
    And creating a macro, or a separate class, for just handling transaction exceptions is simply not as good.
    Sometimes, more control will be needed, but most of the time people will be just fine with a “smart default”

    Log in to Reply
    Logger
    November 25, 2016 at 3:33 pm
    Michael, I believe that it is necessary to fix the core.

    Correction in X ++ code has a number of problems:
    1. Not all developers know about the bug discussed and probably will never know. (Moreover, few people know ). It is therefore not correct to be a bug. The bug will remain forever. For example, I posted information about it on axforum in 2011, but so far very few people know about it in Russia. (axforum – the only Russian-language resource on DAX)
    2. Most of the developers since Ax3, accustomed that the transaction is atomic and X ++ supports it (Acid). But this is not the case.
    3. Documentation X ++ explicitly recommends that you can use Exception- All. It is not enough to recommend the developers to correct the X ++ code. At the same time it is necessary to fix the system documentation.
    4. In reality, now the developer is forced to constantly think about the danger and cumbersome to write exception handling. This is extremely inconvenient and prevents focus on the essentials. X ++ in this case, – makes life difficult, not easier.
    5. Find all the places in the code that must be very difficult to correct. (They are still many standard code)

    In the case of kernel patches:
    1. Developers do not need to be aware of this bug and a lot of problems with uninformed programmers disappears.
    2. Acid still been supporting X ++. Software really gets what he expects and what it was used since ax3
    3. It is not necessary to correct the documentation – the system will start to work as well as recommended in the documentation.
    4. Exception handling code is again very simple. There is no need to think about the complex. (This is the main reason. Most developers will simply be happy by this simplification.)
    5. Do not look for a lot of places in the code X ++ to fix. It is enough to put a fix for the kernel! I think you will agree that it is much easier than to review and analyze all of the source X ++ code. In addition, a bug often works, But users and programmers do not notice it. For them, the situation looks like in the middle of a transaction to commit. Now all is corrected itself.

    And further:
    I do not know the kernel source code, but I think that to correct the mistake would not be very difficult as the kernel, when exiting ttsBegin/ttsCommit scope – changes ttslevel, ie kernel monitors the level of the transaction. I think that we should not reduce the tts-counter by 1, but the transaction counter is to be reseted to 0. It will be safer. (In this case, we do not renounce the TTSunrollbackable exceptions. Only fix their behavior)

    Log in to Reply
    Metin Emre
    November 28, 2016 at 4:57 pm
    Logger,

    I think this’s not a bug. first record saved, while second is saving there’s an error occurs and you decided what’ll do in catch. You can ignore the bug, or you can throw an error and break the transcaction. That’s logical. However I never noticed this behaviour until today and in first a bit shocked.

    Thanks for article…

    Log in to Reply
    AGolubkov
    November 25, 2016 at 8:41 am
    thanks for post.
    It’s stupid core bug, as i see.
    That ‘trick’ don’t happens in old version of AX. I check it on AX4, and when raise UpdateConflict on the line of credit update, code goes to catch in postList method. And therefore, without any violation of transactional integrity.

    Log in to Reply
    Michael Fruergaard Pontoppidan
    November 25, 2016 at 9:09 am
    Thanks! That is very useful information. I’ve tried this in AX2012 R3 and AX7. Both have this problem. Does anyone have an AX2009 they can try on?

    Log in to Reply
    Logger
    November 25, 2016 at 12:49 pm
    I’ve tried in ax2009.
    Bug exists and caused some problem in production.

    I described this here
    Развалились InventSum - InventTrans

    Here is brief translation
    The job :
    static void Job791_15(Args _args)
    {
    #define.recIdTrans(5637151827) // here must be existsing recid from InventTrans.
    InventTrans InventTrans;
    InventSum InventSum;
    ;
    try
    {
    ttsBegin;
    try
    {
    InventTrans = InventTrans::findRecId(#recIdTrans, true);
    InventTrans.Qty += 1.0;
    InventTrans.Update();
    breakpoint;
    }
    catch
    {
    info(“Catched inner catch”);
    }
    ttsCommit;
    }
    catch
    {
    info(“Catched outer catch”);
    }

    info(strFMT(“InventTrans.RowCount() = %1, InventTrans.Qty = %2, InventSum : %3 “,
    InventTrans.RowCount(),
    InventTrans.Qty,
    con2str(Global::buf2Con(InventSum::find(InventTrans.ItemId, InventTrans.inventDimId)), “; “)
    ));

    }

    How to use:
    1. Edit job, setting in line 3 the recid of existing InventTrans.
    2. Open 2 ax client.
    3. Run the job on the first client and wait when it stops in debugger.
    4. Run the job on the second client and wait. Client must hang. It wil be blocked by first client.
    5. Press F5 in debugger for first client.
    6. Press F5 in debugger for second client.
    So, both jobs are to be performed.
    As result, InventTrans.Qty increases by 1.0 and the apropriate column in InventSum increases by 2.0

    Logger
    November 25, 2016 at 1:10 pm
    It can not be reproduced in ax3 because it does not support Exception::UpdateConflict and Exception:uplicateKeyException

    Michael Fruergaard Pontoppidan
    November 26, 2016 at 7:41 am
    Hi AGolubkov,
    What is the build number of AX4 you tried on? I looked at the compiler/interpreter code in AX4 vs AX2009 and found nothing indicating the behavior should be different.

    UPDATE: I tried on AX4SP1, it also has this problem. I’m going to assume this has been an issue since the new exception types got introduced.
    There is an image showing it here: https://msdnshared.blob.core.windows...buggingTTS.png

    Thanks
    Michael

    Log in to Reply
    AGolubkov
    February 2, 2017 at 8:33 am
    Hi Michael,
    sorry for late reply, I didn’t see your question.

    I’ve tried it on application Dynamics AX 4.0 SP2 4.0.2501.116, and hadn’t any problems with trasactional integrity in this example.
    https://drive.google.com/file/d/0B1U...ew?usp=sharing

    AGolubkov
    February 2, 2017 at 8:38 am
    Hi Michael,
    soryy for late reply, I didn’t see your question…

    I’ve tried this example in DAX application Dynamics AX 4.0 SP2 4.0.2501.116.
    And I didn’t see any problem with transactional integrity.
    https://drive.google.com/open?id=0B1...Dk4NnRRSm5lLUk

    Denis
    November 24, 2016 at 5:47 pm
    Nice post. Do you have any ideas how to find such ‘unsafe’ code in the existing application

    Log in to Reply
    Michael Fruergaard Pontoppidan
    November 24, 2016 at 7:12 pm
    Hi Denis, there are many ways to find them. As I had all source files on disk (I’m on the AX7 stack), I did a “findstr /i /s /C:”catch” *.* >catches.txt”. Then I used Notepad++ to filter out the noise. Within a few minutes I had the list of potential problems, and I then started the investigation in Visual Studio. There are other (and likely more efficient) ways – but this did the job for me.

    -Michael

    Log in to Reply
    Denis
    November 25, 2016 at 4:19 am
    By the way – can you register this issue as a bug?
    It seems more logical that the language should guarantee that what is written between ttsbegin.. ttscommit should be considered as one transaction, not just the way that developer write code

    Michael Fruergaard Pontoppidan
    November 25, 2016 at 9:13 am
    Let us assume this is a compiler bug, and it get resolved as a KB. How long will it take before all systems in the world are patched? Wouldn’t it be better to write code that is reliable with and without the fix. Another complicating factor is that all X++ must be recompiled for an eventual fix to have any effect. Rest assured the X++ compiler team is informed; and this thread will be updated with news.

    Logger
    November 29, 2016 at 1:36 pm
    Hi Michael.

    >> Never use catch “all” without explicitly catching updateConflict and duplicateKey exceptions.

    I suggest to create a BestPractice check, verifying before the code “Catch-ALL” is always exists “catch (Exception::UpdateConflict)” and “catch (Exception:uplicateKeyException)”

    What we get:
    1. There will be found all the places that need to be fixed, with the global compilation.
    2. If one uses Exception incorrectly, BP check warns programmer. So, code will be fixed immediatly.
    3. In the future, it will be easier to maintain the correct code.

    Martin Dráb
    November 24, 2016 at 3:55 pm
    A nice demonstration of an ugly problem.

    It also shows that X++ / AX lacks tools for developers to handle exceptions correctly – catch clauses in “What does reliable code look like?” take more than 30 lines. Also, X++ doesn’t have a statement for re-throwing the same exception (as throw; does in C#).

    There are also other issues with exception handling. If anybody can break my exception handling logic just by calling my method from inside a transaction, it means that I can’t really rely on it. Together with the fact that exceptions don’t carry additional details such as a type (something like FileNotFoundException) it leads to a complete absence of code for recovering from errors (people either let the process stop, or handle all exceptions, as in this post).

    Generics and better exceptions are my top two things I would like to see in X++.

    Log in to Reply
    Michael Fruergaard Pontoppidan
    November 24, 2016 at 7:23 pm
    Hi Martin, As this post points out, it is actually possible to handle exceptions in X++ correctly, and it is possible to write a method that cannot be broken by a consumer. My hope is that this post will enable X++ developers to do so.

    I do agree that handling of exceptions is much too hard and much too verbose in X++; and there is plenty of room for improvements.

    -Michael

    Log in to Reply
    Max
    November 25, 2016 at 11:19 pm
    So it is a bug in the core which is at least 5 years old and we have a solution now: “Just avoid holding it in that way”. Cheers

    Martin Dráb
    November 27, 2016 at 2:57 am
    Micheal, I mean that if I want, for example, all errors to be logged and then continue processing (or use any other arbitrary error handling logic), all my logic is ignored when somebody calls my logic from inside of another transaction.
    Your post doesn’t show how to address it and it can’t, because it’s not possible. Your post is clearly about exceptions that don’t have this problem, because they actually *can* be handled inside transactions.

    Michael Fruergaard Pontoppidan
    November 28, 2016 at 7:57 am
    Thanks for the elaboration, Martin. The original design of transaction and exception handling in AX was to ensure data integrity – at the cost of flexibility. Giving back the full flexibility as you are requesting without risking data integrity is an interesting challenge. (As this post shows).
    Запись от Logger размещена 03.06.2019 в 09:43 Logger is offline
  2. Старый комментарий
    Запись от Logger размещена 03.09.2019 в 14:53 Logger is offline
 


Рейтинг@Mail.ru
Часовой пояс GMT +3, время: 14:29.