1

I have a little problem. In this code, always there is catch{} section firing. Even if any exception is thrown. I checked in debugger and no exception is THROWN but somehow code from catch{} is firing and it transfers me to google.com. If I comment the code from catch{}, Page is running fine. Someone know why is that? It makes me mad. Thanks

protected void Button5_Click(object sender, EventArgs e)
    {
        if (Page.IsValid == true)
        {
            try
            {
                conn = new MySqlConnection("Server=localhost;Port=3306;Database=ewidencja;Uid=webuser;Pwd=web1;");
                conn.Open();
                MySqlDataAdapter mda = new MySqlDataAdapter();
                mda.SelectCommand = new MySqlCommand("select id from pacjenci where pesel='" + Session["pesel"].ToString() + "';", conn);
                int id_pacjenta = (int)mda.SelectCommand.ExecuteScalar();
                int id_lekarza=Int32.Parse(DropDownList1.SelectedValue);
                mda.InsertCommand = new MySqlCommand("insert into planowane_wizyty (id_pacjenta, id_lekarza, data_wizyty) values(" + id_pacjenta + ", " + id_lekarza + ", '" + Calendar1.SelectedDate.ToString().Substring(0,10)+" "+ ListBox1.SelectedItem.Value + "');", conn);
                if (mda.InsertCommand.ExecuteNonQuery() == 1)
                    Response.Redirect("wizyty.aspx");
                else
                    Response.Redirect("info.aspx");
            }
            catch (Exception ex)
            {
                Response.Redirect("http://www.google.com");
            }
        }
    }
Paweł Adamczyk
  • 223
  • 3
  • 6
  • 14
  • 1
    There must be an exception.. have you put a breakpoint on your `Response.Redirect("http://www.google.com");` and then examined `ex` when it gets hit? – Darren Wainwright Jan 08 '13 at 20:03
  • 1
    An exception _must_ be thrown somewhere. Did you check the stack trace on the `ex` object? It should tell you what threw. – Oded Jan 08 '13 at 20:03
  • 1
    Take out the try/catch and all redirects, set the debugger to break on all exceptions, and then see what happens. – Tim M. Jan 08 '13 at 20:06
  • It says: `[System.Threading.ThreadAbortException] = {Unable to evaluate expression because the code is optimized or a native frame is on top of the call stack.}` – Paweł Adamczyk Jan 08 '13 at 20:06
  • 1
    There you go: http://stackoverflow.com/questions/2777105/response-redirect-causes-system-threading-threadabortexception – Tim M. Jan 08 '13 at 20:08

2 Answers2

3

Response.Redirect can throw a ThreadAbortException. This then hits the outer exception handler, triggering the second Response.Redirect.

See Why Response.Redirect causes System.Threading.ThreadAbortException?

More importantly, this is one reason that data access code should not be mixed so tightly with UI behavior. Debugging such code is difficult, unit testing is near impossible, and reusability is low.

It also looks like your query is being constructed via string concatenation, which is vulnerable to SQL injection. Parameterize the query instead.

Community
  • 1
  • 1
Tim M.
  • 53,671
  • 14
  • 120
  • 163
  • Correct me if I am wrong, but I believe the overload OP is using **always** throws an error. There is an overload that accepts a boolean value to not abort the current thread, but that opens up a whole new can of worms. – Shai Cohen Jan 08 '13 at 21:00
  • 1
    Correct, that overload throws ThreadAbortException. There is more detail in the question I linked. This is why I suggest not coupling business/data logic so closely with the UI. A proper implementation could return a boolean, a status, a complex type, etc. which allowed the UI to redirect to the correct page. There would be no need to wrap that logic in a try/catch. – Tim M. Jan 08 '13 at 21:03
  • I completely agree. I just wanted to clarify that specific point. – Shai Cohen Jan 08 '13 at 21:08
0

The overload of Response.Redirect you are using will always try and stop the current thread. This causes the exception you are seeing. System.Threading.ThreadAbortException

There is an overload available: Response.Redirect(string url, bool endResponse) that allows you to control whether to end the current thread using the endResponse parameter.

All that being said, you can catch this specific error and ignore it. The suggestion below is just one of a few solutions you can implement. It all depends on what you are trying to do.

protected void Button5_Click(object sender, EventArgs e)
    {
        if (Page.IsValid == true)
        {
            try
            {
                conn = new MySqlConnection("Server=localhost;Port=3306;Database=ewidencja;Uid=webuser;Pwd=web1;");
                conn.Open();
                MySqlDataAdapter mda = new MySqlDataAdapter();
                mda.SelectCommand = new MySqlCommand("select id from pacjenci where pesel='" + Session["pesel"].ToString() + "';", conn);
                int id_pacjenta = (int)mda.SelectCommand.ExecuteScalar();
                int id_lekarza=Int32.Parse(DropDownList1.SelectedValue);
                mda.InsertCommand = new MySqlCommand("insert into planowane_wizyty (id_pacjenta, id_lekarza, data_wizyty) values(" + id_pacjenta + ", " + id_lekarza + ", '" + Calendar1.SelectedDate.ToString().Substring(0,10)+" "+ ListBox1.SelectedItem.Value + "');", conn);
                if (mda.InsertCommand.ExecuteNonQuery() == 1)
                    Response.Redirect("wizyty.aspx");
                else
                    Response.Redirect("info.aspx");
            }
            catch (System.Threading.ThreadAbortException)
            {
                //do nothing. This is an expected error stemming from Response.Redirect
            }
            catch (Exception ex)
            {
                Response.Redirect("http://www.google.com");
            }
        }
    }
Shai Cohen
  • 6,074
  • 4
  • 31
  • 54