How to Recover from a DbUpdateException With EF Core

Dec 5, 2022 aspnetcore efcore suggest edit

There are cases where recovery from an Entity Framework Core (EF Core) DbUpdateException is possible if you play your cards right. Play them wrong and the result is heartbreak and tears as every call to SaveChangesAsync rethrows the same exception.

The following story examines one example of heartache and tears. This story is based on actual events. Only the names, locations, and code have been changed to protect the guilty. All code samples have been simplified for brevity.

Robot recovering in an ice bath

My company, A Serious Business, Inc. (the real name), has a product called Abbot (name is also real), a Slack app that helps customer success/support teams keep track of conversations within Slack and support more customers with less effort.

As a Slack App, it needs to ingest a lot of Slack events. At a high level, we have a Controller that receives Slack events over HTTP, saves a SlackEvent entity to the database, and then sends a message to a background service (Hangfire) to process the event.

A while ago, one of our developers, we’ll call him Bill Maack (any similarity to persons real or imagined is entirely “coincidental”), wrote code to run our bot pipeline on incoming Slack events. The code looks something like this (_db is an instance of AbbotContext, our DbContext derived class).

// This code runs in a background Hangfire job.
public async Task ProcessEventAsync(int id) {
    var nextEvent = (await _db.SlackEvents.FindAsync(id))
        ?? throw new InvalidOperationException($"Event not found: {id}");
    
    try {
        await RunPipelineAsync(nextEvent);
    }
    catch (Exception e) {
        nextEvent.Error = e.ToString();
    }
    finally {
        nextEvent.Completed = DateTime.UtcNow;
        await _db.SaveChangesAsync();
    }
}

This code worked well enough for a long time, but we noticed that our logs would have the occasional uncaught DbUpdateException. More odd was it was thrown in the finally block at the line where _db.SaveChangesAsync() is called. The exception message indicated a unique constraint violation for a user record. This was confusing because we’re trying to save a SlackEvent which is totally unrelated to saving a user. So Phil…I mean Bill, ~~ignored it for a while in a confused stupor~~immediately jumped into action. Our Application Insights logs indicated the exception was a result of the following SQL query:

UPDATE "SlackEvents" SET "Completed" = @p0
WHERE "Id" = @p1;
INSERT INTO "Users" ("SlackId")
VALUES (@p2)
RETURNING "Id";

The plot thickens! Why are there two queries? Well we need a little more background about our app to understand that.

Here’s our User entity, stripped down to its skivvies for brevity.

public class User {
    public int Id { get; set; }

    // Slack User Id looks something like U012BILJMAK
    public string SlackId { get; set; } = null!;
    //...
}

The OnModelCreating method of AbbotContext configures a unique constraint on the SlackId column of User:

protected override void OnModelCreating(ModelBuilder modelBuilder) {
    base.OnModelCreating(modelBuilder);

    modelBuilder.Entity<User>()
        .HasIndex(i => i.SlackId)
        .IsUnique();
    // ...
}

At some point in our pipeline, we try to retrieve a user by their slack id, and if we don’t find one, we create a new one. The code looks something like this:

public async Task<User> GetUserBySlackIdAsync(string slackId) {
    var user = await _db.Users.SingleOrDefaultAsync(u => u.SlackId == slackId);
    if (user is null) {
        user = new User {
            SlackId = slackId,
        };
        await _db.Users.AddAsync(user);
        await _db.SaveChangesAsync();
    }

    return user;
}

If you squint, you can see a race condition lying in wait to prey on the hapless developer in this code. After the code checks if the User exists, another thread could have created the User. So when we try to save the User, we get a DbUpdateException because the unique constraint is violated.

Here’s Bill’s first naive attempt to fix this:

public async Task<User> GetUserBySlackIdAsync(string slackId) {
    var user = await _db.Users.SingleOrDefaultAsync(u => u.SlackId == slackId);
    if (user is null) {
        user = new User {
            SlackId = slackId,
        };
        await _db.Users.AddAsync(user);
        try {
            await _db.SaveChangesAsync();
        }
        catch (DbUpdateException) {
            // Maybe the user already exists? If so, return that user.
            user = await _db.Users.SingleOrDefaultAsync(u => u.SlackId == slackId);
            if (user is null) {
                throw;
            }
        }
    }

    return user;
}

After testing it a bit, Bill called it a day. A job well done. How little did Silly Billy know. As it turns out, this was the source of the weird DbUpdateExceptions at the top of our callstack. Do you see the issue?

The problem is that when the DbUpdateException is thrown, the attempt to insert the new User record is still in the EF change tracker. So any subsequent calls to SaveChangesAsync will continue to try and save the User instance and throw the same exception.

The solution is to detach the User instance from the change tracker after the exception is thrown. We can do this in a generic way by detaching all the relevant entries reported by the exception like so:

    catch (DbUpdateException e) {
        // ...
        foreach (var entry in e.Entries) {
            entry.State = EntityState.Detached;
        }
    }

However, in a rare flash of wisdom, Bill opted to be very specific in this case. It’s unlikely but possible that some other entity caused this exception. That would violate our expectations so we’d want this to throw in that case. Since we have a specific entity we’re trying to create, we should only detach that element.

_db.Entry(user).State = EntityState.Detached;

Here’s that code in context.

public async Task<User> GetUserBySlackIdAsync(string slackId) {
    var user = await _db.Users.SingleOrDefaultAsync(u => u.SlackId == slackId);
    if (user is null) {
        user = new User {
            SlackId = slackId,
        };
        await _db.Users.AddAsync(user);

        try {
            await _db.SaveChangesAsync();
        }
        catch (DbUpdateException) {
            // Maybe the user already exists? If so, return that user.
            var existing = await _db.Users.SingleOrDefaultAsync(u => u.SlackId == slackId);
            if (existing is null) {
                throw;
            }
            _db.Entry(user).State = EntityState.Detached;
            user = existing;
        }
    }

    return user;
}

And that solved the immediate problem. But we’re not done yet. There’s a problem with the exception handling. A unique constraint violation is not the only reason EF might throw a DbUpdateException. And what if it’s a violation for another table? Also, isn’t it a bit fragile that our top-level processing code could throw because the DbContext is in a weird state? Yes. Yes it is fragile.

Stay tuned for the further adventures of our intrepid developer, Bill Maack, as he continues to iterate on this code to make it more robust and tries to be less of a pain in the ass to his team.

Found a typo or mistake in the post? suggest edit

Comments

7 responses

Thomas Freudenberg • December 6th, 2022
Ahem, in the last code snippet, ~Phil~ Bill wants to detach the new User instance, not that one returned by the second SingleOrDefaultAsync call, right?
Steve • December 6th, 2022
Shouldn’t the detach be done on the user variable before you assigned it with the database value? I would guess that they both be different instances in memory
haacked • December 6th, 2022
Thomas and Steve, you’re absolutely correct! This is what happens when I try to transcribe code and simplify it for brevity. I just posted an update with the correct code. We want to query to a new variable. We only want to detach if we’re going to recover.
Przemyslaw • December 7th, 2022
DbContext is not supposed to be thread safe. Why are allowing your repository method to be executed concurrently from multiple threads?
haacked • December 7th, 2022

Why are allowing your repository method to be executed concurrently from multiple threads?

I’m not. The background task gets its own instance of DbContext injected. I’ll explain this in a follow-up post.
Przemyslaw • December 8th, 2022
Got it. That is at db level. I recommend you should try experimenting with (UPDLOCK, SERIALIZABLE) hints to achieve atomicity and somehow embed them into EF config. https://sqlperformance.com/2020/09/locking/upsert-anti-pattern
Ken McCormack • November 30th, 2023
Thanks for the post Phil! I held back a PR on DbContext pooling - because of this concern. I couldn’t find any doc on whether EF Core auto-cleans. The symptom first revealed itself in a set of integration tests, which used the same DbContext for a series of fixture creates… (a null string in a non-nullable property will do it), so yes, if the DbContext gets an “unsavable” entity added to its state, all the tests in the suite go red.

(So, would this leak away valid DbContexts in the pool, and cause random failures, slowly bringing the system down??)

So, another callout is that every consumer should not need to handle this… it’s an orthogonal concern. Analogous to this are services setting common fields like “CreatedBy” and “CreatedDate” (DRY fail code smell). To get around this, we override SaveChangesAsync in our DbContext, and pass in a ‘caller’ object, so fields like CreatedBy / UpdatedBy, CreatedDate, UpdatedDate are set centrally (if the type implements a custom interface “IAuditedEntity”) so, every repository was doing this, it’s now one piece of code - see below -

public virtual async Task SaveChangesAsync(UserId callerId) { var now = _dateTimeService.UtcNow; // so that tests are deterministic
// ChangeTracker is null in a unit test - has to be an integ test foreach (var changedEntity in ChangeTracker.Entries()) { if (changedEntity.Entity is IAuditedEntity entity) { switch (changedEntity.State) { case EntityState.Added: entity.CreatedDate = now; entity.UpdatedDate = null; entity.CreatedBy = callerId; entity.UpdatedBy = null; break; case EntityState.Modified: Entry(entity).Property(x => x.CreatedBy).IsModified = false; Entry(entity).Property(x => x.CreatedDate).IsModified = false; entity.UpdatedDate = now; entity.UpdatedBy = callerId; break; } } }
// todo - add Detach in a try-catch here, to clean the pooled context return await base.SaveChangesAsync(); }

So, puzzle… how to write an integration test to replicate a parallel unit of work performing a breaking insert - it needs to interleave with the main flow’s read and insert…

I will try inheriting the DbContext (“TestDbContext”), and adding that into the container, casting it first to the base type. The service should resolve the test context. Its overload for SaveChangesAsync would create a new DbContext (if some test condition is met), and perform an insert, then continue with the service’s unit of work… calling base.SaveChangesAsync(). This will throw a DbUpdateException, the DbContext should catch, detach the failing entity, and throw. In the test, calling SaveChangesAsync a second time on the same DbContext instance (adding a new, valid, entity), should work.

(That’s a theory!)