Gfrktyl

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.

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.

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

        ]);

    }



}

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