Skip to content

Polecat and Fisher discard ANY unique-constraint violation, not just a duplicate inbox row #4565

Description

@jeremydmiller

PolecatIntegration.Configure and FisherIntegration.Configure both register a Discard() rule for
duplicate incoming messages that matches on the provider's error number alone, with no check of which
table raised it.

src/Persistence/Wolverine.Polecat/PolecatIntegration.cs:

// Duplicate incoming messages - SQL Server uses unique constraint violations
options.OnException<Microsoft.Data.SqlClient.SqlException>(e =>
    {
        // Unique key violation on incoming table
        return e.Number == 2627 || e.Number == 2601;
    })
    .Discard();

src/Persistence/Wolverine.Fisher/FisherIntegration.cs:

options.OnException<Microsoft.Data.Sqlite.SqliteException>(e =>
        e.SqliteExtendedErrorCode == 1555 || e.SqliteExtendedErrorCode == 2067)
    .Discard();

The comment in the Polecat one says "Unique key violation on incoming table". The predicate does not check
that, and cannot: 2627 / 2601 is any primary-key or unique-key violation anywhere in the transaction.

Why this matters

Compare the Marten twin, MartenIntegration.Configure, which scopes to both the table and the constraint:

options.OnException<MartenCommandException>(e =>
    {
        if (e.InnerException is PostgresException pg)
        {
            return pg.TableName == DatabaseConstants.IncomingTable && pg.ConstraintName.IsNotEmpty() &&
                   pg.ConstraintName.StartsWith("pkey");
        }
        return false;
    })
    .Discard();

On Polecat and Fisher, a handler that commits a document with a duplicate natural key, a unique index
violation on the application's own table, or any other PK collision gets its message silently
discarded
. Not retried, not dead-lettered — Discard() acknowledges it and moves on. The work never
happened and there is no dead letter to find it in, which is the single hardest failure mode to diagnose
after the fact.

This is latent today and gets worse with #4505's commit-time deduplication race handling, which
deliberately classifies a specific table's unique violation. A blanket rule sitting underneath one that
carefully scopes itself defeats the point.

Suggested fix

Scope both rules to the incoming-envelopes table the way Marten's does. The per-provider pieces already
exist:

  • MessageDatabase.IsUniqueConstraintViolation(Exception) / isExceptionFromDuplicateEnvelope — but note
    these are internal to Wolverine.RDBMS and protected respectively, which is presumably why both
    integrations hand-inlined raw error numbers instead. Widening the seam is part of the work.
  • SQL Server's SqlException does not carry a table name, but the message text does
    (SqlServerMessageStore.isExceptionFromDuplicateEnvelope already matches on
    "Violation of PRIMARY KEY constraint" / "Violation of UNIQUE KEY constraint"), and the constraint
    name embedded in it can be matched against the incoming table's PK name.
  • SQLite likewise reports UNIQUE constraint failed: <table>.<column> in the message.

A negative-control test is worth having on each: a handler that commits an application document with a
duplicate key must dead-letter, not vanish.

Found while implementing #4505.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions