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

QueryAsync no longer throws SQL Exception from 1.50.* #563

Closed
OliverRC opened this issue Jul 15, 2016 · 11 comments
Closed

QueryAsync no longer throws SQL Exception from 1.50.* #563

OliverRC opened this issue Jul 15, 2016 · 11 comments

Comments

@OliverRC
Copy link

OliverRC commented Jul 15, 2016

We've run into an issue with the latest 1.50.* release when raising errors in SQL.
When calling QueryAsync on a stored procedure that selects a result set and then does a RaiseError no SQLException is thrown anymore. This used to be the behavior in 1.42.0.

It's almost as if the error is being "swallowed" somewhere.

Example code:

private static async Task<IList<Example>> TestAsync()
{
  const string connectionString = "User ID=sa;Password=password;Data Source=TestServer;Initial Catalog=DapperTest;Persist Security Info=False";

  using (var connection = new SqlConnection(connectionString))
  {
    connection.Open();
    // In 1.42.0 this throws an error. From 1.50.* it doesn't.
    return (await connection.QueryAsync<Row>("dbo.TestProc", commandType: CommandType.StoredProcedure)).ToList();
  }
}

Object

public class Example
{
  public override string ToString()
  {
    return SomeValue.ToString();
  }

  private int SomeValue { get; set; }
}

Stored Procedure

CREATE PROCEDURE dbo.TestProc
AS BEGIN

  DECLARE @SomeValue1 INT = 123;
  DECLARE @SomeValue2 INT = 456;
  DECLARE @SomeValue3 INT = 789;

  -- If RAISEERROR is done before the SELECT, an exception will be received in C#, using Dapper 1.50.1.
  --RAISERROR('before select', 16, 1);

  SELECT @SomeValue1 SomeValue
    UNION ALL
    SELECT @SomeValue2 SomeValue
    UNION ALL
    SELECT @SomeValue3 SomeValue;

  -- If RAISEERROR is done after the SELECT, no exception will be received in C#, using Dapper 1.50.1
  RAISERROR('after select', 16, 1);

END;
@NickCraver NickCraver changed the title QueryAsync not longer throws SQL Exception from 1.50.* QueryAsync no longer throws SQL Exception from 1.50.* Jul 16, 2016
@OliverRC
Copy link
Author

Is there any update on this? It is currently an issue in our production codebase which we'd like to get resolved.

@JeremyTCD
Copy link

This issue is related to #525 . It's a pretty big issue for me too, using a dirty fix in some SqlMapper functions for now:

else if ((row & Row.FirstOrDefault) == 0) 
{  
    // Extra read retrieves SqlException
    await reader.ReadAsync(cancel);
    ThrowZeroRows(row);
}

@OliverRC
Copy link
Author

Any update on this?

@OliverRC
Copy link
Author

OliverRC commented Sep 8, 2016

The lack of response on this issue is rather unsettling. We use Dapper heavily in some high value products and yet we don't seem to be receiving any support for a genuine issue. Please could I get some feedback?

mgravell added a commit that referenced this issue Sep 8, 2016
@mgravell
Copy link
Member

mgravell commented Sep 8, 2016

I'll take a look; I have a repro here - investigating.

Edit: Oops; forgot to add the raiserror in my repro! But: looking, is the point

The reality is that we can't always jump immediately onto everything - all time on this project has to come from somewhere, and we are only human, and often have many other things going on, whether coding or otherwise. PRs are more than welcome :)

@mgravell
Copy link
Member

mgravell commented Sep 8, 2016

I think the issue here is the CommandBehavior.SingleResult optimization; this appears to be causing the TDS parser to short-circuit immediately after the first result (which is perhaps reasonable). Removing the SingleResult on line 240 of SqlMapper.Async.cs makes it work as you want. There has been some recent discussion on what to do with these optimizations; will need to consider this as part of that. discussion.

Edit: I realize this is exactly what @JeremyTCD said above - this is mainly by way of confirmation.

mgravell added a commit that referenced this issue Sep 8, 2016
…SingleResult=disallowed - but expose options; this fixes issues like #563 automatically, and allows the performance aspect  of #554 to be fixed via a global setting
@mgravell
Copy link
Member

mgravell commented Sep 8, 2016

Candidate fix has been pushed; I want a second set of eyeballs before I merge it, though - any chance @NickCraver ? #602

mgravell added a commit that referenced this issue Sep 9, 2016
Make CommandBehavior optimizations more configurable
@OliverRC
Copy link
Author

Awesome stuff guys, it there a NuGet release we can test it with?

@NickCraver
Copy link
Member

Fixed in the current release - closing out to cleanup

@ebadullah
Copy link

I am using Dapper 1.50.2.0 and still facing the issue. when try to execute with ExecuteAsync using StoredProcedure.
CAn you please guide me towards the fix you provided somewhere?

@noyb-ch
Copy link

noyb-ch commented Jan 29, 2019

I am using 1.50.5 and this is still an issue with ExecuteScalar/ExecuteScalarAsync.
Was the fix only applied on the Query and QueryAsync methods? It seems to work properly there.

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

No branches or pull requests

6 participants