Reduced necessity for OpenIdException handling outside of the library.

The AuthenticationStatus.Failed enum value is now used to represent failed attempts that previously threw an OpenIdException.
The OpenIdTextBox control (and derivative OpenIdLogin) has seen some breaking changes as the result of this change, as noted below.
BREAKING CHANGES:
Old behavior:
* exceptions fired the OnError event.
* SetupRequired fired the OnFailed event.

New behavior:
* exceptions fire the OnFailed event (new signature, causes compile error)
* SetupRequired fires the new OnSetupRequired event.
* OnError event no longer exists (causes compile error)

samples/RelyingPartyPortal/login.aspx

@@ -13,7 +13,7 @@
 	<cc1:OpenIdLogin ID="OpenIdLogin1" runat="server" CssClass="openid_login" RequestCountry="Request"
 		RequestEmail="Request" RequestGender="Require" RequestPostalCode="Require" RequestTimeZone="Require"
 		RememberMeVisible="True" PolicyUrl="~/PrivacyPolicy.aspx" TabIndex="1" OnLoggedIn="OpenIdLogin1_LoggedIn"
-		OnCanceled="OpenIdLogin1_Canceled" OnError="OpenIdLogin1_Error" OnFailed="OpenIdLogin1_Failed" />
+		OnCanceled="OpenIdLogin1_Canceled" OnFailed="OpenIdLogin1_Failed" OnSetupRequired="OpenIdLogin1_SetupRequired" />
 	<asp:CheckBox ID="immediateCheckBox" runat="server" Text="Immediate mode" />
 	<br />
 	<asp:Label ID="loginFailedLabel" runat="server" EnableViewState="False" Text="Login failed"

samples/RelyingPartyPortal/login.aspx.cs

@@ -15,13 +15,17 @@ protected void Page_Load(object sender, EventArgs e) {
 	protected void OpenIdLogin1_LoggedIn(object sender, OpenIdEventArgs e) {
 		State.ProfileFields = e.ProfileFields;
 	}
-	protected void OpenIdLogin1_Error(object sender, ErrorEventArgs e) {
+	protected void OpenIdLogin1_Failed(object sender, OpenIdEventArgs e) {
 		loginFailedLabel.Visible = true;
-		loginFailedLabel.Text += ": " + e.ErrorMessage;
+		loginFailedLabel.Text += ": " + e.Response.Exception.Message;
 	}
 	protected void OpenIdLogin1_Canceled(object sender, OpenIdEventArgs e) {
 		loginCanceledLabel.Visible = true;
 	}
+	protected void OpenIdLogin1_SetupRequired(object sender, OpenIdEventArgs e) {
+		loginFailedLabel.Text = "You must log into your Provider first to use Immediate mode.";
+		loginFailedLabel.Visible = true;
+	}
 
 	protected void yahooLoginButton_Click(object sender, ImageClickEventArgs e) {
 		OpenIdRelyingParty openid = new OpenIdRelyingParty();
@@ -30,11 +34,4 @@ protected void yahooLoginButton_Click(object sender, ImageClickEventArgs e) {
 		// We don't listen for the response from the provider explicitly
 		// because the OpenIdLogin control is already doing that for us.
 	}
-
-	protected void OpenIdLogin1_Failed(object sender, OpenIdEventArgs e) {
-		if (e.Response.Status == AuthenticationStatus.SetupRequired) {
-			loginFailedLabel.Text = "You must log into your Provider first to use Immediate mode.";
-			loginFailedLabel.Visible = true;
-		}
-	}
 }

src/DotNetOpenId.Test/RelyingParty/AuthenticationResponseTests.cs

@@ -95,7 +95,7 @@ void resign(ref Uri uri) {
 			uri = builder.Uri;
 		}
 
-		[Test, ExpectedException(typeof(OpenIdException))]
+		[Test]
 		public void ReturnToMismatchDetection() {
 			ProtocolVersion version = ProtocolVersion.V20;
 			Protocol protocol = Protocol.Lookup(version);
@@ -104,15 +104,17 @@ public void ReturnToMismatchDetection() {
 			// which should cause a failure because the return_to argument
 			// says that parameter is supposed to be there.
 			removeQueryParameter(ref assertion, returnToRemovableParameter);
-			var result = new OpenIdRelyingParty(store, assertion).Response.Status;
+			var response = new OpenIdRelyingParty(store, assertion).Response;
+			Assert.AreEqual(AuthenticationStatus.Failed, response.Status);
+			Assert.IsNotNull(response.Exception);
 		}
 
 		/// <summary>
 		/// Verifies that the RP rejects signed assertions by an OP that makes up a
 		/// claimed Id that was not part of the original request, and that the OP
 		/// has no authority to assert positively regarding.
 		/// </summary>
-		[Test, ExpectedException(typeof(OpenIdException))]
+		[Test]
 		public void SpoofedClaimedIdDetection_20() {
 			ProtocolVersion version = ProtocolVersion.V20;
 			Protocol protocol = Protocol.Lookup(version);
@@ -136,7 +138,9 @@ public void SpoofedClaimedIdDetection_20() {
 			resign(ref assertion); // resign changed URL to simulate a contrived OP for breaking into RPs.
 
 			// (triggers exception) "... you're in trouble up to your ears."
-			var result = new OpenIdRelyingParty(store, assertion).Response.Status;
+			var response = new OpenIdRelyingParty(store, assertion).Response;
+			Assert.AreEqual(AuthenticationStatus.Failed, response.Status);
+			Assert.IsNotNull(response.Exception);
 		}
 
 	}

src/DotNetOpenId/DotNetOpenId.csproj

@@ -63,6 +63,7 @@
     <Compile Include="RelyingParty\ApplicationMemoryStore.cs" />
     <Compile Include="RelyingParty\DirectRequest.cs" />
     <Compile Include="RelyingParty\DirectResponse.cs" />
+    <Compile Include="RelyingParty\FailedAuthenticationResponse.cs" />
     <Compile Include="RelyingParty\Fetcher.cs" />
     <Compile Include="RelyingParty\FetchResponse.cs" />
     <Compile Include="RelyingParty\IAuthenticationRequest.cs" />

src/DotNetOpenId/RelyingParty/AuthenticationResponse.cs

@@ -13,8 +13,7 @@ public enum AuthenticationStatus {
 		/// </summary>
 		Canceled,
 		/// <summary>
-		/// The authentication failed because the provider refused to send
-		/// authentication approval.
+		/// The authentication failed because an error was detected in the OpenId communication.
 		/// </summary>
 		Failed,
 		/// <summary>
@@ -54,6 +53,12 @@ internal AuthenticationResponse(AuthenticationStatus status, ServiceEndpoint pro
 		/// </summary>
 		public AuthenticationStatus Status { get; private set; }
 		/// <summary>
+		/// Details regarding a failed authentication attempt, if available.
+		/// This will only be set if <see cref="Status"/> is <see cref="AuthenticationStatus.Failed"/>,
+		/// but may sometimes by null in this case as well.
+		/// </summary>
+		public Exception Exception { get; private set; }
+		/// <summary>
 		/// An Identifier that the end user claims to own.
 		/// </summary>
 		public Identifier ClaimedIdentifier {
@@ -135,7 +140,7 @@ internal static AuthenticationResponse Parse(IDictionary<string, string> query,
 			} else {
 				throw new OpenIdException(string.Format(CultureInfo.CurrentUICulture,
 					Strings.InvalidOpenIdQueryParameterValue,
-					protocol.openid.mode, mode));
+					protocol.openid.mode, mode), query);
 			}
 		}
 

src/DotNetOpenId/RelyingParty/FailedAuthenticationResponse.cs

@@ -0,0 +1,29 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace DotNetOpenId.RelyingParty {
+	class FailedAuthenticationResponse : IAuthenticationResponse {
+		public FailedAuthenticationResponse(Exception exception) {
+			Exception = exception;
+		}
+
+		#region IAuthenticationResponse Members
+
+		public IDictionary<string, string> GetExtensionArguments(string extensionTypeUri) {
+			return new Dictionary<string, string>();
+		}
+
+		public Identifier ClaimedIdentifier {
+			get { return null; }
+		}
+
+		public AuthenticationStatus Status {
+			get { return AuthenticationStatus.Failed; }
+		}
+
+		public Exception Exception { get; private set; }
+
+		#endregion
+	}
+}

src/DotNetOpenId/RelyingParty/IAuthenticationResponse.cs

@@ -23,5 +23,10 @@ public interface IAuthenticationResponse {
 		/// The detailed success or failure status of the authentication attempt.
 		/// </summary>
 		AuthenticationStatus Status { get; }
+		/// <summary>
+		/// Details regarding a failed authentication attempt, if available.
+		/// This will be set if and only if <see cref="Status"/> is <see cref="AuthenticationStatus.Failed"/>.
+		/// </summary>
+		Exception Exception { get; }
 	}
 }

src/DotNetOpenId/RelyingParty/OpenIdRelyingParty.cs

@@ -116,15 +116,19 @@ bool isAuthenticationResponseReady {
 				return true;
 			}
 		}
-		AuthenticationResponse response;
+		IAuthenticationResponse response;
 		/// <summary>
 		/// Gets the result of a user agent's visit to his OpenId provider in an
 		/// authentication attempt.  Null if no response is available.
 		/// </summary>
 		public IAuthenticationResponse Response {
 			get {
 				if (response == null && isAuthenticationResponseReady) {
-					response = AuthenticationResponse.Parse(query, store, request);
+					try {
+						response = AuthenticationResponse.Parse(query, store, request);
+					} catch (OpenIdException ex) {
+						response = new FailedAuthenticationResponse(ex);
+					}
 				}
 				return response;
 			}

src/DotNetOpenId/RelyingParty/OpenIdTextBox.cs

@@ -395,36 +395,29 @@ public override bool EnableTheming
 		}
 		#endregion
 
-		protected override void OnLoad(EventArgs e)
-		{
+		protected override void OnLoad(EventArgs e) {
 			base.OnLoad(e);
 
 			if (!Enabled) return;
-			try
-			{
-				var consumer = new OpenIdRelyingParty();
-				if (consumer.Response != null)
-				{
-					switch (consumer.Response.Status) {
-						case AuthenticationStatus.Canceled:
-							OnCanceled(consumer.Response);
-							break;
-						case AuthenticationStatus.Authenticated:
-							OnLoggedIn(consumer.Response);
-							break;
-						case AuthenticationStatus.SetupRequired:
-						case AuthenticationStatus.Failed:
-							OnFailed(consumer.Response);
-							break;
-						default:
-							throw new InvalidOperationException("Unexpected response status code.");
-					}
+			var consumer = new OpenIdRelyingParty();
+			if (consumer.Response != null) {
+				switch (consumer.Response.Status) {
+					case AuthenticationStatus.Canceled:
+						OnCanceled(consumer.Response);
+						break;
+					case AuthenticationStatus.Authenticated:
+						OnLoggedIn(consumer.Response);
+						break;
+					case AuthenticationStatus.SetupRequired:
+						OnSetupRequired(consumer.Response);
+						break;
+					case AuthenticationStatus.Failed:
+						OnFailed(consumer.Response);
+						break;
+					default:
+						throw new InvalidOperationException("Unexpected response status code.");
 				}
 			}
-			catch (OpenIdException ex)
-			{
-				OnError(ex);
-			}
 		}
 
 		protected override void OnPreRender(EventArgs e) {
@@ -465,9 +458,9 @@ protected void PrepareAuthenticationRequest() {
 				Request.Mode = ImmediateMode ? AuthenticationRequestMode.Immediate : AuthenticationRequestMode.Setup;
 				if (EnableRequestProfile) addProfileArgs(Request);
 			} catch (WebException ex) {
-				OnError(ex);
+				OnFailed(new FailedAuthenticationResponse(ex));
 			} catch (OpenIdException ex) {
-				OnError(ex);
+				OnFailed(new FailedAuthenticationResponse(ex));
 			}
 		}
 
@@ -537,6 +530,8 @@ UriBuilder getResolvedRealm(string realm)
 		public event EventHandler<OpenIdEventArgs> LoggedIn;
 		protected virtual void OnLoggedIn(IAuthenticationResponse response)
 		{
+			if (response == null) throw new ArgumentNullException("response");
+			Debug.Assert(response.Status == AuthenticationStatus.Authenticated);
 			var loggedIn = LoggedIn;
 			OpenIdEventArgs args = new OpenIdEventArgs(response);
 			if (loggedIn != null)
@@ -546,21 +541,20 @@ protected virtual void OnLoggedIn(IAuthenticationResponse response)
 					response.ClaimedIdentifier.ToString(), UsePersistentCookie);
 		}
 
-		#endregion
-		#region Error handling
 		/// <summary>
-		/// Fired when a login attempt fails or is canceled by the user.
+		/// Fired when a login attempt fails.
 		/// </summary>
-		[Description("Fired when a login attempt fails or is canceled by the user.")]
-		public event EventHandler<ErrorEventArgs> Error;
-		protected virtual void OnError(Exception errorException)
+		[Description("Fired when a login attempt fails.")]
+		public event EventHandler<OpenIdEventArgs> Failed;
+		protected virtual void OnFailed(IAuthenticationResponse response)
 		{
-			if (errorException == null)
-				throw new ArgumentNullException("errorException");
+			if (response == null) throw new ArgumentNullException("response");
+			Debug.Assert(response.Status == AuthenticationStatus.Failed);
+			Exception errorException = response.Exception;
 
-			var error = Error;
-			if (error != null)
-				error(this, new ErrorEventArgs(errorException.Message, errorException));
+			var failed = Failed;
+			if (failed != null)
+				failed(this, new OpenIdEventArgs(response));
 		}
 
 		/// <summary>
@@ -570,20 +564,26 @@ protected virtual void OnError(Exception errorException)
 		public event EventHandler<OpenIdEventArgs> Canceled;
 		protected virtual void OnCanceled(IAuthenticationResponse response)
 		{
+			if (response == null) throw new ArgumentNullException("response");
+			Debug.Assert(response.Status == AuthenticationStatus.Canceled);
+
 			var canceled = Canceled;
 			if (canceled != null)
 				canceled(this, new OpenIdEventArgs(response));
 		}
 
 		/// <summary>
-		/// Fired when an authentication attempt fails at the OpenID Provider.
+		/// Fired when an Immediate authentication attempt fails, and the Provider suggests using non-Immediate mode.
 		/// </summary>
-		[Description("Fired when an authentication attempt fails at the OpenID Provider.")]
-		public event EventHandler<OpenIdEventArgs> Failed;
-		protected virtual void OnFailed(IAuthenticationResponse response) {
-			var failed = Failed;
-			if (failed != null)
-				failed(this, new OpenIdEventArgs(response));
+		[Description("Fired when an Immediate authentication attempt fails, and the Provider suggests using non-Immediate mode.")]
+		public event EventHandler<OpenIdEventArgs> SetupRequired;
+		protected virtual void OnSetupRequired(IAuthenticationResponse response) {
+			if (response == null) throw new ArgumentNullException("response");
+			Debug.Assert(response.Status == AuthenticationStatus.SetupRequired);
+			// Why are we firing Failed when we're OnSetupRequired?  Backward compatibility.
+			var setupRequired = SetupRequired;
+			if (setupRequired != null)
+				setupRequired(this, new OpenIdEventArgs(response));
 		}
 
 		#endregion
@@ -602,6 +602,7 @@ internal OpenIdEventArgs(Identifier claimedIdentifier) {
 		/// (whether that attempt was successful or not).
 		/// </summary>
 		internal OpenIdEventArgs(IAuthenticationResponse response) {
+			if (response == null) throw new ArgumentNullException("response");
 			Response = response;
 			ClaimedIdentifier = response.ClaimedIdentifier;
 			ProfileFields = SimpleRegistrationFieldValues.ReadFromResponse(response);
@@ -622,12 +623,4 @@ internal OpenIdEventArgs(IAuthenticationResponse response) {
 		/// </summary>
 		public SimpleRegistrationFieldValues ProfileFields { get; private set; }
 	}
-	public class ErrorEventArgs : EventArgs {
-		public ErrorEventArgs(string errorMessage, Exception errorException) {
-			ErrorMessage = errorMessage;
-			ErrorException = errorException;
-		}
-		public string ErrorMessage { get; private set; }
-		public Exception ErrorException { get; private set; }
-	}
 }