I have a trigger on Account and Opportunity - both do a callout to the Google Geocoding API and then update itself using a future method. Now I also want to send an email before the update if certain criteria are met - and suddenly all my unit tests fail.

System.CalloutException: You have uncommitted work pending. Please commit or rollback before calling out

I'm super confused as to why this happens when all I want is to send an email. Any ideas?

Also, it should be noted that this is not a duplicate of System.CalloutException: You have uncommitted work pending as my error only occurs when I'm trying to send an email. So my question is specifically why it only occurs when I add that email sending code.

  • 2
  • Can you provide your test class so we can see what is happening. David Reed's answer is great but it doesn't address some other issues that can pop up in test classes. – gNerb Nov 20 at 20:20
  • Not a duplicate - but thanks for noticing. My question is why I only get the error when sending emails. I'll see if I can strip down the code to the minimum needed. – Semmel Nov 20 at 20:59
  • I agree it's not a duplicate. David reeds answer explains why it happens when you send an email. My answer explains why it might be erroring in a test as opposed to working successfully outside of tests. – gNerb Nov 20 at 21:05

Some operations are "DML-ish", meaning they persist something to the database to be committed at the end of the transaction, just not a standard DML operation on an sObject. Enqueuing Batch Apex, for example, is a DML-ish operation. (This is a term I made up, by the way; I don't know if there's official terminology to describe this type of operation).

It's documented that

[Outbound] email is not sent until the Apex transaction is committed.

Because this is persisting something (the email send attempt) until the transaction commits, it has the same effect on later callouts as regular DML - that is, it blocks them due to the uncommitted work that's in flight.

The key is ordering - ensuring that all your callouts happen first, followed by all database mutation.

Here's a simple demonstration. Note that this is not in test context, where we can't send outbound email anyway. Given this class:

public class TestQ240040 {
    public static void runTest() {
        Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage();

        email.setToAddresses(new List<String>{'david@ktema.org'});
        email.setPlainTextBody('Text');
        email.setSaveAsActivity(false);
        Messaging.sendEmail(
            new List<Messaging.Email> {email}
        );

        // Now make a callout
        Http http = new Http();
        HttpRequest req = new HttpRequest();
        req.setEndpoint('https://www3.septa.org/hackathon/Arrivals/Market%20East/100');
        req.setMethod('GET');

        HttpResponse res = http.send(req);
    }
}

If in Anonymous Apex you should do

TestQ240040.runTest();

You get back

System.CalloutException: You have uncommitted work pending. Please commit or rollback before calling out

If you but reverse the order of the callout and email send, all is well:

public class TestQ240040 {
    public static void runTest() {
        Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage();

        email.setToAddresses(new List<String>{'david@ktema.org'});
        email.setPlainTextBody('Text');
        email.setSaveAsActivity(false);

        // Now make a callout
        Http http = new Http();
        HttpRequest req = new HttpRequest();
        req.setEndpoint('https://www3.septa.org/hackathon/Arrivals/Market%20East/100');
        req.setMethod('GET');

        HttpResponse res = http.send(req);

        Messaging.sendEmail(
            new List<Messaging.Email> {email}
        );
    }
}

No exception, and the email gets delivered as expected.

Now, I would actually expect a different error from your unit tests (since you cannot send outbound email there), but I think the above is the core issue leading to the exception you're discussing here.

  • 1
    He does mention a test class so maybe provide some information on test.starttest and test.stoptest and better testing methodologies. This may not be an ordering issue. – gNerb Nov 20 at 20:19
  • So this would eventually explain why this happens only when sending an email. Since it's DM-ish (nice term - btw) this might be blocking the callout. But how this is executed is still a mystery to me. I'll investigate further. – Semmel Nov 20 at 21:02
  • @Semmel, I think it would be helpful if you could provide a skeleton of your code to help clarify where the true issue is. Since you can't send email in test context anyway, if you guard your sendEmail() call with if (!Test.isRunningTest()), you may be able to demonstrate that this is the issue. – David Reed Nov 20 at 21:07
  • I also suffered through the dreaded Pending Work Exception due to a "DML-ish" operation. For me it was enqueuing a job in the Apex Flex Queue. This sounds like it might be similar. – John Thompson Nov 20 at 21:24
  • Yes,, you cannot do callout after enqueing a Queuable job :( – Pranay Jaiswal Nov 20 at 21:55

Note: writing this answer with plenty of detail on good testing habits to be used as a re-usable answer. Will be updated over time.

Some of the things I do when writing tests are:

  • Turn off triggers: we have a custom setting we can use to turn triggers on and off and make sure triggers are always off during tests. We keep most of the code for triggers inside utility classes that can be tested using the rest of the principals below. The trigger test itself only tests for bulkification by creating records in bulk and firing the trigger, it doesn't test for any of the outputs. Those are covered in the utils tests.
  • Test only 1 method at a time: If you follow the single-purpose principal your method should be relatively short and succinct and they should accept few inputs and return an output. Using this approach makes testing super easy because you don't test the full user story, only what the one method is supposed to be doing. If you use a different method for your email and future calls you can test these two methods separately to ensure they work as expected. This also splits up the code into different apex contexts which will not only fix your current issue, but leads me to my next point
  • Test for apex contexts: When testing I advise making sure that you test only what will realistically happen in a single context. For instance, Visualforce pages will often communicate with the server more than once while being used. You do not want to test all of the actions leading up to a save request in the same context as the save request. You can use different test methods to help split contexts or you can use test.starttest and test.stoptest to control where the context starts and ends. test.starttest and test.stoptest also act as signals for firing async operations such as callouts.
  • Tests are static: use static variables to store lists of records to operate on and static initialization code to initialize these lists. The static initialization code fires before every test so this allows you to group repetitive tasks/queries into a single place to be re-used automatically at the beginning of every test.
  • Use data factories to create records. I've seen many different approaches to this, my current organization opted to use a single class for all methods but other orgs I worked with created 1 factory per object and a generic factory that did non-object related methods (such as shutting off triggers) or grouping objects together into single methods (such as creating all of the records required to create an opportunity+quote). What ever your preference, factories keep your tests clean. Note: most factories return records to be inserted as opposed to inserting the records within the factory. This is obviously not as doable when grouping methods together to automatically create dependencies but its important to know which methods actually insert the records and which return them and the benefits and pitfalls of both.

Sample Class:

public class AccountUtils {
    public static void doAsync(List<Account> accounts) {
        try {
            doAsync(new Map<Id, Account>(accounts));
        } catch (Exception ex) {
            system.debug('accounts do not have Ids, move to before update or after triggers');
        }
    }

    public static void doAsync(Map<id, Account> accounts) {
        doCallout(accounts.keySet());
        sendEmail(accounts.values()); // Notice this is after the callout
                                      // in accordance with David Reed's answer
    }

    public static void sendEmail(List<Account> newAccounts) {
        public List<Messaging.singleEmailMessage> messages = new List<Messaging.singleEmailMessage>();

        for (Account a : newAccounts) {
            if (a.something) {
                Messaging.singleEmailMessage newMessage = new Messaging.singleEmailMessage();
                newMessage.Subject = 'Something happened';
                messages.add(newMessage);
            }
        }

        if (!messages.isEmpty()) {
            Messaging.send(messages);
        }
    }

    @future(callout=true)
    public static void sendCallout(Set<Id> recordIds) {
        CalloutService service = new CalloutService();
        service.namedCred = 'Imaginary Named Credential';
        service.doAuth();

        httpRequest req = service.newReq();
        req.setBody(JSON.serialize(recordIds));
        req.setMethod('POST');

        httpResponse res = service.send(req);

        /*
            Moar stuff
        */
    }
}

Sample test:

@isTest
public class AccountUtilsTest {
    public Map<Id, Account> accounts {get; set;}

    static {
        accounts = getAccounts();
    }

    @testSetup
    public static void testSetup() {
        accounts = TestDataFactory.newAcconts(3);
        insert accounts;
    }

    @isTest
    public static void sendEmailTest() {
        test.startTest();
        accountUtils.sendEmail(accounts.values());
        test.stopTest();

        system.assertEquals(1, Limits.getEmailInvocations());
    }

    @isTest
    public static void sendCalloutTest() {
        test.startTest();
        accountUtils.sendCallout(accounts.keySet());
        test.stopTest();

        system.assert(/* Whatever you want to test */);
    }

    @isTest
    public static void doAsync() {
        test.startTest();
        accountUtils.doAsync(accounts.values());
        test.stopTest();

        // Since we are testing the outputs in other locations we do not need asserts here
    }

    public static List<Account> getAccounts() {
        return new Map<id, Account>([
            SELECT Id, OtherField
            FROM Account
        ]);
    }

}
  • Thank you for your valuable input - I will incorporate this as much as I can. Sadly most of the tests and a couple lines of the code are legacy stuff so I'm not sure what I can fix without refactoring. – Semmel Nov 20 at 20:50
  • 2
    I've been wanting to get an answer like this out there with good testing habits that I follow for my list of re-usable answers. I'm going to be working on and updating this answer for a while so that I can re-use it in the future. Feel free to keep an eye on the updates. – gNerb Nov 20 at 20:54
  • +1 from me for great content. – David Reed Nov 20 at 21:08
  • an additional approach to unit testing (and avoiding DML of setup records) is to use the Trailhead Enterprise patterns and apexmocks as illustrated in Andrew Fawcett's "Force.com Enterprise Architecture" – cropredy Nov 20 at 21:18

Get ready for world's ugliest solution. Use at own risk

As we cannot send an email and then do a callout in the same transaction as per @David's answer, we have to use a hack.

Yes we cannot do a callout after DML, but we can do callout after a callout.

Why not make your send email as a callout instead? yes I am talking about execute anon rest endpoint.

You have to convert your Send email code as a string and escape the '

Here is the same code that allows you to do so.

String executAnonCode ='Messaging.SingleEmailMessage[] emailMessages = new Messaging.SingleEmailMessage[] {};'+
'Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage();'+
'String htmlBody = \'My boday\';'+
'email.setHtmlBody( htmlBody );'+
'email.setTargetObjectId( \'0030D000004cVFJ\');'+
'email.setSaveAsActivity( false );'+
'email.setSubject( \'Subject\' );'+
'emailMessages.add( email );'+
'Messaging.sendEmail( emailMessages );';

        Http http = new HTTP();
HttpRequest httpRequest = new HttpRequest();
httpRequest.setMethod('GET');
httpRequest.setEndPoint(URL.getSalesforceBaseUrl().toExternalForm() + '/services/data/v40.0/tooling/executeAnonymous?anonymousBody='+EncodingUtil.urlEncode(executAnonCode,'UTF-8'));
httpRequest.setHeader('Authorization', 'Bearer ' + UserInfo.getSessionId());// We will use session id from anothe rest call
HttpResponse httpResp = http.send(httpRequest);

System.debug(httpResp.getBody() + httpResp.getStatusCode());

//Do your other callouts
  • 1
    I can repro the exception even with setSaveAsActivity(false). This would fire it anyway though even if the core email send didn't! – David Reed Nov 20 at 21:49
  • Let me check by printing ,System.debug(Limits.getDmlStatements()); after sending email – Pranay Jaiswal Nov 20 at 21:49
  • Yes you are right @DavidReed , It prints that DML rows and DML statements as 0 and still doesnt allow me to do callout.. So it does a DML when sending email, :( – Pranay Jaiswal Nov 20 at 21:53
  • Updated answer to demonstrate use of a hack which I tested and works. – Pranay Jaiswal Nov 20 at 22:10
  • I... I love it. It's so crazy. +1. – David Reed Nov 20 at 22:14

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.