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

Show detail text in PostgresException.Mesaage #2151

Closed
austindrenski opened this issue Sep 11, 2018 · 3 comments · Fixed by #3437
Closed

Show detail text in PostgresException.Mesaage #2151

austindrenski opened this issue Sep 11, 2018 · 3 comments · Fixed by #3437
Assignees
Milestone

Comments

@austindrenski
Copy link
Contributor

austindrenski commented Sep 11, 2018

Moved from npgsql/efcore.pg#633:

@phrohdoh wrote:

I do not know if this belongs in this repo or npgsql proper.


When I run dotnet ef database update in my project I get the following:

Npgsql.PostgresException (0x80004005): 42804: foreign key constraint "external_auths_user_id_fkey" cannot be implemented
  at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
  at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext() in C:\projects\npgsql\src\Npgsql\NpgsqlConnector.cs:line 1032
--- End of stack trace from previous location where exception was thrown ---
  at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming) in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 467
  at Npgsql.NpgsqlDataReader.NextResult() in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 332
  at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1219
  at Npgsql.NpgsqlCommand.ExecuteNonQuery(Boolean async, CancellationToken cancellationToken) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1042
  at Npgsql.NpgsqlCommand.ExecuteNonQuery() in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1025
  at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues)
  at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteNonQuery(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues)
  at Microsoft.EntityFrameworkCore.Migrations.MigrationCommand.ExecuteNonQuery(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues)
  at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteNonQuery(IEnumerable`1 migrationCommands, IRelationalConnection connection)
  at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration)
  at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.UpdateDatabase(String targetMigration, String contextType)
  at Microsoft.EntityFrameworkCore.Design.OperationExecutor.UpdateDatabase.<>c__DisplayClass0_1.<.ctor>b__0()
  at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
42804: foreign key constraint "external_auths_user_id_fkey" cannot be implemented

Which includes the error message and code but does not include the detail message that pg spits out when I execute my raw SQL directly:

ERROR:  foreign key constraint "external_auths_user_id_fkey" cannot be implemented
DETAIL:  Key columns "user_id" and "id" are of incompatible types: bigint and text.

Of course users could go to the error code appendix if they know it exists but including the detail message here would have been beneficial, I believe.


Versions of my dependencies (may or may not be useful):

   <PackageReference Include="Microsoft.EntityFrameworkCore" Version="2.1.2" />
   <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="2.1.2" />
   <PackageReference Include="Npgsql" Version="4.0.3" />

Related

/// <summary>
/// The primary human-readable error message. This should be accurate but terse.
/// </summary>
/// <remarks>
/// Always present.
/// </remarks>
[PublicAPI]
public string MessageText { get; set; }
/// <summary>
/// An optional secondary error message carrying more detail about the problem.
/// May run to multiple lines.
/// </summary>
[PublicAPI]
public string Detail { get; set; }

/// <summary>
/// Gets a the PostgreSQL error message and code.
/// </summary>
public override string Message => SqlState + ": " + MessageText;

@austindrenski
Copy link
Contributor Author

austindrenski commented Sep 11, 2018

@roji Would it be possible to make the verbosity of PostgresException configurable?

Something like:

enum NpgsqlVerbosity
{
    Minimal,
    Detail,
    Verbose
}

NpgsqlConnection.GlobalConfiguration.Verbosity = NpgsqlVerbosity.Detail;

@roji
Copy link
Member

roji commented Sep 11, 2018

I seem to remember a discussion around this recently, but can't find the issue.

I definitely agree there's value in surfacing more of PostgresException's fields in the exception message. Specifically for Detail, we could consider just adding it in parentheses after the main message (if it is given).

We could also just duplicate PostgreSQL's error format, including all the different values. The only issue here is that it would potentially be a big, multiline exception message and these don't seem common/standard in the .NET world. This is where I'm leaning towards.

Finally, we could make this configurable via a formatter, whereby the user would provide a lambda accepting a PostgresException instance and returning the message text. This would leave it up to the user, although I think this is a good case where the library should just do the right thing, rather than providing configurable knobs.

Any comments?

@austindrenski
Copy link
Contributor Author

We could also just duplicate PostgreSQL's error format, including all the different values.

I also lean toward just surfacing the full PostgreSQL format.

From the EF Core perspective, I could imagine that being confusing for some users, but I think that is outweighed by the insight it would give those familiar with the backend. Plus it would make stack traces much more valuable for issue triage.

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

Successfully merging a pull request may close this issue.

3 participants