Make sure to "return" before assigning an "onclick" event in code-behind

I often find myself forgetting the trivial... especially when it comes to assigning onclick attributes to a control in code-behind processing. A perfect example is in the OnRowDataBound event for a GridView where I want to assign an cleint-side event handler to a Delete button to show a JavaScript Confirm (OK/Cancel) before the data hits the bit-bucket.

GridView Event Handler in code-behind:
protected void gridview_OnRowDataBound(object sender, GridViewRowEventArgs e)
{
if (e.Row.RowType == DataControlRowType.DataRow)
{
// Add a JavaScript warning event handler to the delete button
LinkButton deleteLinkButton = (LinkButton)e.Row.FindControl("btnDelete");
if (deleteLinkButton != null)
deleteLinkButton.Attributes.Add("onclick", "return ConfirmDelete('" + whateverString
+ "');");
}
}


JavaScript Function in ASPX Mark-Up:

function ConfirmDelete(someData)
{
var msg = "Are you sure you wish to delete this item?\n\n("
+ someData + ")";
if (confirm(msg) == true)
return true;
else
return false;
}

Static Connection String Cause of DataReader Failure - SQLDataReader IndexOutOfRange

Background:
For the past four months, I have been performing fixes and enhancements to a data management application that my client had developed by a third-party vendor. It is a relatively data-intensive ASP.NET application that utilizes the ASP.NET Membership Provider, as well as several custom data access classes. All query logic is done through stored procedures deployed on a SQL Server 2005 database. This application is intended to support concurrent users, but unforrtunately we frequently encounter exceptions such as SQLDataReader ... IndexOutOfRange. This error typically occurs if you try to reference columns that were not returned by a query. In our particular case, we verified that the stored procedures always returned the named columns, and that the DataReader.HasRows always evaluated to true. The DataReader logic was nested within a try/catch block like:

try
{
conn.Open();
SqlDataReader reader = sqlCmd.ExecuteReader(CommandBehavior.SingleRow);
string userName = "";
reader.Read();

if (reader.HasRows)
{
userName = GetDBValue(reader["UserName"]);
}
reader.Close();

return userName;
}
catch (SqlException ex)
{
throw new ApplicationException("An error has occured ...");
}
finally
{
conn.Close();
}

I was lucky enough to find a pattern to consistenly reproduce the error condition, which was essentially opening two browsers and executing the same query. This is a sad story given the application fails under the load of two concurrent users. I spent several days tuning the SQL Server 2005 database, and modifying application code to minimize calls to the database such as persisting data into ViewState. After these efforts, we still experienced the dreaded DataReader errors.

Solution:
I came across this large thread on MSDN entitled "SQLDataReader.GetOrdinal() fails rarely with IndexOutOfRange" This article offers several suggestions such as handling DataReader errors, disabling connection pooling and avoid persisting a SQL data connection in a static instance. I took the first, and least invasive route by disabling connection pooling in the connection string located in Web.Config. Unfortunately, this not only impacted the performance, but I still observed the same DataReader errors.

I took the next path by determing how the SQL connection as being utilized. Bingo! I saw that a utility class contained the following:

public class DBUtils
{
protected static SqlConnection conn = null;

protected static SqlConnection GetConn()
{
if (ConnectionString == null)
ConnectionString = WebConfigurationManager.ConnectionStrings
["ApplicationConnectionString"].ConnectionString;
return new SqlConnection(ConnectionString);
}
}

Calls were being made to the above function as:

conn = GetConn();

I modified the above to use a more traditional approach as follows:

SqlConnection conn = new SqlConnection(Common.AppConfig.SqlConnectionString);

Note: AppConfig.SQLConnection string is a public accessor in the application's Common namespace to the connnection string stored in Web.Config.

By creating a new connection string instance instead of relying on the static instance, the application is now able to handle concurrent queries without throwing any DataReader exceptions. I can not say that this will be the ultimate solution for this application, but at least it's a proven stop-gap measure for now.

References:
http://social.msdn.microsoft.com/Forums/en-US/adodotnetdataproviders/thread/74a5db87-c63f-4386-b317-e848dedb2cd9).