Merge pull request #297 from OneNoteDev/bug/o365-sign-in-errors

Bug/o365 sign in errors

Author: leakymemory

src/scripts/clipperUI/clipper.tsx

@@ -576,41 +576,43 @@ class ClipperClass extends ComponentBase<ClipperState, {}> {
 		let handleSignInEvent = new Log.Event.PromiseEvent(Log.Event.Label.HandleSignInEvent);
 
 		this.setState({ userResult: { status: Status.InProgress } });
-		type ErrorObject = { correlationId?: string, error: string, errorDescription: string };
-		Clipper.getExtensionCommunicator().callRemoteFunction(Constants.FunctionKeys.signInUser, { param: authType, callback: (data: UserInfo | ErrorObject) => {
+
+		type ErrorObject = {
+			updateReason: UpdateReason,
+			correlationId?: string,
+			error: string,
+			errorDescription: string
+		};
+
+		Clipper.getExtensionCommunicator().callRemoteFunction(Constants.FunctionKeys.signInUser, {
+			param: authType, callback: (data: UserInfo | ErrorObject) => {
 			// For cleaner referencing
+			// TODO: This kind of thing should go away as we move the communicator to be Promise based.
 			let updatedUser = data as UserInfo;
 			let errorObject = data as ErrorObject;
 
-			// Unexpected errors
-			let errorPrefix = AuthType[authType] + "; ";
-			let error: string;
-			if (!updatedUser) {
-				error = errorPrefix + "The " + Constants.FunctionKeys.signInUser + " remote function incorrectly returned an undefined object";
-			} else if (errorObject.error || errorObject.errorDescription) {
-				// Something went wrong on the auth server
-				error = errorPrefix + errorObject.error + ": " + errorObject.errorDescription;
-				handleSignInEvent.setCustomProperty(Log.PropertyName.Custom.CorrelationId, errorObject.correlationId);
-			}
+			let errorsFound = errorObject.error || errorObject.errorDescription;
+			if (errorsFound) {
+				errorObject.errorDescription = AuthType[authType] + ": " + errorObject.error + ": " + errorObject.errorDescription;
 
-			if (error) {
-				handleSignInEvent.setStatus(Log.Status.Failed);
-				handleSignInEvent.setFailureInfo({ error: error });
+				this.state.setState({ userResult: { status: Status.Failed, data: errorObject } });
 
-				errorObject.errorDescription = error;
-				this.state.setState({ userResult: { status: Status.Failed, data: updatedUser } });
+				handleSignInEvent.setStatus(Log.Status.Failed);
+				handleSignInEvent.setFailureInfo({ error: errorObject.errorDescription });
+				handleSignInEvent.setCustomProperty(Log.PropertyName.Custom.CorrelationId, errorObject.correlationId);
 
 				Clipper.logger.logUserFunnel(Log.Funnel.Label.AuthSignInFailed);
 			}
 
 			let userInfoReturned = updatedUser && !!updatedUser.user;
 			if (userInfoReturned) {
-				// Sign in succeeded
 				Clipper.storeValue(ClipperStorageKeys.hasPatchPermissions, "true");
 				Clipper.logger.logUserFunnel(Log.Funnel.Label.AuthSignInCompleted);
 			}
+
 			handleSignInEvent.setCustomProperty(Log.PropertyName.Custom.UserInformationReturned, userInfoReturned);
-			handleSignInEvent.setCustomProperty(Log.PropertyName.Custom.SignInCancelled, !error && !userInfoReturned);
+			handleSignInEvent.setCustomProperty(Log.PropertyName.Custom.SignInCancelled, !errorsFound && !userInfoReturned);
+
 			Clipper.logger.logEvent(handleSignInEvent);
 		}});
 	}

src/scripts/clipperUI/panels/signInPanel.tsx

@@ -100,9 +100,6 @@ class SignInPanelClass extends ComponentBase<SignInPanelState, SignInPanelProps>
 	debugInformation() {
 		if (this.signInErrorDetected() && this.state.debugInformationShowing) {
 			return <div id={Constants.Ids.signInErrorDebugInformation}>
-				<span id={Constants.Ids.signInErrorDebugInformationDescription} style={Localization.getFontFamilyAsStyle(Localization.FontFamily.Light)}>
-					{this.props.clipperState.userResult.data.errorDescription}
-				</span>
 				<div id={Constants.Ids.signInErrorDebugInformationContainer} style={Localization.getFontFamilyAsStyle(Localization.FontFamily.Light)}>
 					<ul id={Constants.Ids.signInErrorDebugInformationList}>
 						<li>{ClientType[this.props.clipperState.clientInfo.clipperType]}: {this.props.clipperState.clientInfo.clipperVersion}</li>

src/scripts/extensions/extensionWorkerBase.ts

@@ -511,6 +511,11 @@ export abstract class ExtensionWorkerBase<TTab, TTabIdentifier> {
 			}).catch((errorObject) => {
 				// Set the user info object to undefined as a result of an attempted sign in
 				this.auth.user.set({ updateReason: UpdateReason.SignInAttempt });
+
+				// Right now we're adding the update reason to the errorObject as well so that it is preserved in the callback.
+				// The right thing to do is revise the way we use callbacks in the communicator and instead use Promises so that
+				// we are able to return distinct objects.
+				errorObject.updateReason = UpdateReason.SignInAttempt;
 				return Promise.reject(errorObject);
 			});
 		});

src/strings.json

@@ -45,7 +45,7 @@
 	"WebClipper.Error.ConflictingExtension": "Your PDF viewer or another extension might be blocking the OneNote Web Clipper. You could temporarily disable the following extension and try clipping again.",
 	"WebClipper.Error.CannotClipPage": "Sorry, this type of page can\u0027t be clipped.",
 	"WebClipper.Error.CookiesDisabled.Line1": "Cookies must be enabled in order for OneNote Web Clipper to work correctly.",
-	"WebClipper.Error.CookiesDisabled.Line2": "Please allow third-party cookies in your browser or add the onenote.com domain as an exception.",
+	"WebClipper.Error.CookiesDisabled.Line2": "Please allow third-party cookies in your browser or add the onenote.com and live.com domains as an exception.",
 	"WebClipper.Error.CorruptedSection": "Your clip can\u0027t be saved here because the section is corrupt.",
 	"WebClipper.Error.GenericError": "Something went wrong. Please try clipping the page again.",
 	"WebClipper.Error.GenericExpiredTokenRefreshError": "Your login session has ended and we were unable to clip the page. Please sign in again.",