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

Issue with addToLoopCount method #19

Open
darko-dj opened this issue Jan 8, 2019 · 5 comments
Open

Issue with addToLoopCount method #19

darko-dj opened this issue Jan 8, 2019 · 5 comments

Comments

@darko-dj
Copy link

darko-dj commented Jan 8, 2019

Hi Kevin,

I believe there's an issue with the addToLoopCount method, but only when the amount of records changed is greater than 200.

It's easier to explain with an example, so let's assume the following:

  • We implemented the framework in our org and we created AccountTriggerHandler than extends TriggerHandler class
  • We have the following code in the constructor, to ensure the handler executes only once:
public AccountTriggerHandler() {
    this.setMaxLoopCount(1);
}

Now if we run a DML on 600 Account records, it will results in 3 Trigger batches (in Trigger context, records are executed in sets of 200):

  • In the first batch of 200 Account, addToLoopCount will add AccountTriggerHandler and set the count to 1
  • In the second batch of 200 Accounts, addToLoopCount will throw an exception, since AccountTriggerHandler was already executed for the first batch. I don't believe this is the correct behaviour. While technically the AccountTriggerHandler was attempted twice (once for each batch), semantically the handler did NOT run twice (it just ran again for another batch). In fact, it didn't even finish the first run (it only processed the first 200 before failing on the next 200).
  • If we decided to catch the Exception instead of throwing it, we could potentially "silence" the trigger handler for the remaining batches. The impact of that could be quite substantial.

Therefore, I don't think we can keep track of execution counts per handler itself. It would need to be per record per handler I believe.

Cheers

@fbouzeraa
Copy link

Hi darko-dj,
Yes, exact ! I didn't see you post before so I posted the same issue, you can see my proposed solution.
In fact the loop control is actually based on handler name which will bloc run completly since second run fo all events the trigger lisen, so we should control loop by handlerName+event.
I joind my correction.
Best regards,

@gvgramazio
Copy link

@fbouzeraa You posted an issue (#27) on a slightly different point. Yours is based on the differentiation of loops based on different trigger operations while op is more concerned about differentiation based on the same trigger operation on more than 200 records.

@sfdevarch17
Copy link

We are facing the same issue where for bulk job processing, after the first chunk of 200 records, the trigger is bypassing (i modified to use TriggerHandler.bypass instead of throwing an exception). I am thinking of two options

  1. either to compare the trigger.new record set in each transaction with the previous chunk (if not null) and then bypass the trigger if it is the same, else reset the max loop count.
  2. check the trigger.new.size in the individual handler, if it is less that 200, only then setMaxLoopCount, else clearMaxLoopCount

i like option 1 better. what do you guys think? any alternate approach?

@darko-dj
Copy link
Author

darko-dj commented Sep 16, 2022 via email

@fbouzeraa
Copy link

Hi @darko-dj yes you'r right my solution does not cover this cas you discuss (many batch of 200 recs). We should base on records ids as we always done before this framework. It would be best if we change the frame work by pass api using this principle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants