From 0f9f6553861b6d3e393eb1200f8dd7abf5f11881 Mon Sep 17 00:00:00 2001 From: Noel Chou Date: Tue, 30 Jan 2024 14:12:01 -0500 Subject: [PATCH] BL-12977 OneDrive error dialog --- DistFiles/localization/en/Bloom.xlf | 4 + .../localization/en/BloomLowPriority.xlf | 9 + .../problemDialog/NotifyDialog.tsx | 73 ++++-- .../problemDialog/ProblemDialog.tsx | 11 +- .../problemDialog/ReportDialog.tsx | 1 - src/BloomBrowserUI/problemDialog/theme.ts | 2 +- src/BloomBrowserUI/utils/linkHandler.ts | 3 +- src/BloomExe/Book/BookInfo.cs | 12 +- src/BloomExe/Collection/CollectionSettings.cs | 6 +- .../CollectionTab/CollectionTabView.cs | 6 +- .../ErrorReporter/HtmlErrorReporter.cs | 11 +- src/BloomExe/MiscUI/StartupScreenManager.cs | 11 +- src/BloomExe/Program.cs | 21 +- src/BloomExe/Utils/FileException.cs | 42 ++++ src/BloomExe/Utils/OneDriveUtils.cs | 181 ++++++++++++++ .../WebLibraryIntegration/BookUpload.cs | 2 +- .../WebLibraryIntegration/BulkUploader.cs | 2 +- .../web/controllers/BookCommandsApi.cs | 1 + .../web/controllers/ProblemReportApi.cs | 232 +++++++++++------- 19 files changed, 491 insertions(+), 139 deletions(-) create mode 100644 src/BloomExe/Utils/FileException.cs create mode 100644 src/BloomExe/Utils/OneDriveUtils.cs diff --git a/DistFiles/localization/en/Bloom.xlf b/DistFiles/localization/en/Bloom.xlf index f294c4ca7cfa..40c4d73b50c1 100644 --- a/DistFiles/localization/en/Bloom.xlf +++ b/DistFiles/localization/en/Bloom.xlf @@ -5138,6 +5138,10 @@ Do you want to go ahead? ID: ReportProblemDialog.WhatDoing This is the label for the text field where the user enters what they were doing at the time of the problem. + + Search online + ID: ReportProblemDialog.SearchOnline + I get it. Do not show this again. ID: SamplePrintNotification.IGetIt diff --git a/DistFiles/localization/en/BloomLowPriority.xlf b/DistFiles/localization/en/BloomLowPriority.xlf index c6d7ffb7d69b..275adbd6761a 100644 --- a/DistFiles/localization/en/BloomLowPriority.xlf +++ b/DistFiles/localization/en/BloomLowPriority.xlf @@ -185,6 +185,15 @@ Bloom was not able to access a microphone. ID: EditTab.Toolbox.TalkingBookTool.MicrophoneAccessProblem + + OneDrive Problem + ID: ReportProblemDialog.OneDriveProblem + The title of a dialog telling the user about a problem with their Microsoft OneDrive. + + + There is a problem with your Microsoft OneDrive which is preventing Bloom from accessing files. + ID: ReportProblemDialog.OneDriveErrorMessage + diff --git a/src/BloomBrowserUI/problemDialog/NotifyDialog.tsx b/src/BloomBrowserUI/problemDialog/NotifyDialog.tsx index eb10c1a727e5..eaa75a703a3e 100644 --- a/src/BloomBrowserUI/problemDialog/NotifyDialog.tsx +++ b/src/BloomBrowserUI/problemDialog/NotifyDialog.tsx @@ -1,4 +1,5 @@ import * as React from "react"; +import { css } from "@emotion/react"; import { Dialog, DialogActions, @@ -14,18 +15,28 @@ import BloomButton from "../react_components/bloomButton"; import { makeTheme, kindParams } from "./theme"; import { useL10n } from "../react_components/l10nHooks"; import { ProblemKind } from "./ProblemDialog"; +import { hookupLinkHandler } from "../utils/linkHandler"; +import { kBloomBlue, kFormBackground } from "../utils/colorUtils"; -export const NotifyDialog: React.FunctionComponent<{ - reportLabel?: string | null; - secondaryLabel?: string | null; - message: string | null; -}> = props => { - const theme = makeTheme(ProblemKind.NonFatal); +export interface INotifyDialogProps { + message?: string | null; // The localized message to notify the user about. + reportLabel?: string | null; // The localized text that goes on the Report button. Omit or pass "" to disable Report button. + secondaryLabel?: string | null; // The localized text that goes on the secondary action button. Omit or pass "" to disable the secondary action button. + detailsBoxText?: string | null; // Localized text to go into a grey details box under the message. Omit or pass "" to not show a details box. + titleOverride?: string | null; // If present, wil be used in place of the dialog title defined for this level in themes.ts + titleL10nKeyOverride?: string | null; // The L10nKey for the titleOverride, if present. + themeOverride?: ProblemKind | null; // If present, will be used in place of the dialog theme defined for this level in themes.ts +} - const englishTitle = kindParams[ProblemKind.NonFatal].title; - const titleKey = kindParams[ProblemKind.NonFatal].l10nKey; +export const NotifyDialog: React.FC = props => { + const theme = makeTheme(props.themeOverride || ProblemKind.NonFatal); + + const englishTitle = props.titleOverride ?? kindParams[ProblemKind.NonFatal].title; + const titleKey = props.titleL10nKeyOverride ?? kindParams[ProblemKind.NonFatal].l10nKey; const localizedDlgTitle = useL10n(englishTitle, titleKey); + React.useEffect(() => hookupLinkHandler(), []); + const getDialog = () => { return ( post("common/closeReactDialog")} + css={css` + a { + color: ${kBloomBlue}; + }`} > {localizedDlgTitle} @@ -45,16 +60,29 @@ export const NotifyDialog: React.FunctionComponent<{ onClick={() => post("common/closeReactDialog")} /> */} - - {/* InnerHTML is used so that we can insert markup like
into the message. */} - - - {getDialogActionButtons()} + + {/* InnerHTML is used so that we can insert markup like
into the message. */} + + {props.detailsBoxText && + + } + + {getDialogActionButtons()} ); }; @@ -111,16 +139,21 @@ export const NotifyDialog: React.FunctionComponent<{ }; const getCloseButton = (): JSX.Element | null => { + const buttonLabel = props.themeOverride === ProblemKind.Fatal ? "Quit" : "Close"; + const l10nKey = + props.themeOverride === ProblemKind.Fatal + ? "ReportProblemDialog.Quit" + : "Common.Close"; return ( { post("common/closeReactDialog"); }} > - Close + {buttonLabel} ); }; diff --git a/src/BloomBrowserUI/problemDialog/ProblemDialog.tsx b/src/BloomBrowserUI/problemDialog/ProblemDialog.tsx index 95743c61aa00..622d889b306d 100644 --- a/src/BloomBrowserUI/problemDialog/ProblemDialog.tsx +++ b/src/BloomBrowserUI/problemDialog/ProblemDialog.tsx @@ -1,6 +1,6 @@ import * as React from "react"; import "./ProblemDialog.less"; -import { NotifyDialog } from "./NotifyDialog"; +import { INotifyDialogProps, NotifyDialog } from "./NotifyDialog"; import { ReportDialog } from "./ReportDialog"; import { WireUpForWinforms } from "../utils/WireUpWinform"; @@ -9,15 +9,10 @@ export enum ProblemKind { Notify = "notify", User = "user", NonFatal = "nonfatal", - Fatal = "fatal" + Fatal = "fatal", } -export const ProblemDialog: React.FunctionComponent<{ - level: ProblemKind; - message: string; // The localized message to notify the user about. - reportLabel?: string; // The localized text that goes on the Report button. Omit or pass "" to disable Report button. - secondaryLabel?: string; // The localized text that goes on the secondary action button. Omit or pass "" to disable the secondary action button. -}> = props => { +export const ProblemDialog: React.FC<{level:ProblemKind} & Partial> = props => { if (props.level === ProblemKind.Notify) { return ; } else { diff --git a/src/BloomBrowserUI/problemDialog/ReportDialog.tsx b/src/BloomBrowserUI/problemDialog/ReportDialog.tsx index 3bc95243c5f3..79fe743d48b4 100644 --- a/src/BloomBrowserUI/problemDialog/ReportDialog.tsx +++ b/src/BloomBrowserUI/problemDialog/ReportDialog.tsx @@ -105,7 +105,6 @@ export const ReportDialog: React.FunctionComponent<{ includeScreenshot }, result => { - console.log(JSON.stringify(result.data)); const failureResponseString = "failed:"; const link = result.data.issueLink; if (link.startsWith(failureResponseString)) { diff --git a/src/BloomBrowserUI/problemDialog/theme.ts b/src/BloomBrowserUI/problemDialog/theme.ts index 11d3af835c24..78a5e622f3e3 100644 --- a/src/BloomBrowserUI/problemDialog/theme.ts +++ b/src/BloomBrowserUI/problemDialog/theme.ts @@ -29,7 +29,7 @@ export const kindParams = { primaryColor: kNonFatalColor, title: "Bloom had a problem", l10nKey: "ReportProblemDialog.NonFatalTitle" - } + }, }; export function makeTheme(kind: ProblemKind): Theme { diff --git a/src/BloomBrowserUI/utils/linkHandler.ts b/src/BloomBrowserUI/utils/linkHandler.ts index 495d025d1036..7cb895e21937 100644 --- a/src/BloomBrowserUI/utils/linkHandler.ts +++ b/src/BloomBrowserUI/utils/linkHandler.ts @@ -13,7 +13,8 @@ const handler = (e: Event) => { if (!href) { return; } - if (href.startsWith("http") || href.startsWith("mailto")) { + if (href.startsWith("http") || href.startsWith("mailto") || href.startsWith('file')) { + console.log('handling link ', href); e.preventDefault(); e.stopPropagation(); postString("link", href); diff --git a/src/BloomExe/Book/BookInfo.cs b/src/BloomExe/Book/BookInfo.cs index e241a22a3698..e4a4b46f5879 100644 --- a/src/BloomExe/Book/BookInfo.cs +++ b/src/BloomExe/Book/BookInfo.cs @@ -745,7 +745,7 @@ public static string InstallFreshInstanceGuid(string bookFolder) /// This method recurses through the folders under 'pathToDirectory' and keeps track of all the unique bookInstanceId /// guids. When a duplicate is found, we will call InstallFreshInstanceGuid(). /// - public static void RepairDuplicateInstanceIds( + public static void CheckForDuplicateInstanceIdsAndRepair( string pathToDirectory, Func okToChangeId ) @@ -1006,7 +1006,15 @@ public static BookMetaData GetRepairedMetaDataWithIdOnly(string metaDataPath) { if (RobustFile.Exists(metaDataPath)) { - var id = GetIdFromDamagedMetaDataString(RobustFile.ReadAllText(metaDataPath)); + string id; + try + { + id = GetIdFromDamagedMetaDataString(RobustFile.ReadAllText(metaDataPath)); + } + catch (IOException error) + { + throw new FileException(metaDataPath, error); + } if (id != null) return new BookMetaData() { Id = id }; } diff --git a/src/BloomExe/Collection/CollectionSettings.cs b/src/BloomExe/Collection/CollectionSettings.cs index 138b561cb059..122b20c395c7 100644 --- a/src/BloomExe/Collection/CollectionSettings.cs +++ b/src/BloomExe/Collection/CollectionSettings.cs @@ -11,6 +11,7 @@ using Bloom.Book; using Bloom.MiscUI; using Bloom.Publish.BloomPub; +using Bloom.Utils; using Bloom.web.controllers; using DesktopAnalytics; using L10NSharp; @@ -479,7 +480,7 @@ public void Load() LoadDictionary(xml, "Palette", ColorPalettes); } - catch (Exception) + catch (Exception originalError) { string settingsContents; try @@ -495,7 +496,8 @@ public void Load() // We used to notify the user of a problem here. // But now we decided it is better to catch at a higher level, at OpenProjectWindow(), else we have two different // error UI dialogs for the same problem. See BL-9916. - throw; + + throw new FileException(SettingsFilePath, originalError); } try diff --git a/src/BloomExe/CollectionTab/CollectionTabView.cs b/src/BloomExe/CollectionTab/CollectionTabView.cs index 62a6018f7acd..ff36f7deca8d 100644 --- a/src/BloomExe/CollectionTab/CollectionTabView.cs +++ b/src/BloomExe/CollectionTab/CollectionTabView.cs @@ -278,19 +278,19 @@ public void ReadyToShowCollections() // However, until we get rid of the old collection tab, it's tricky to move it, so I've just // duplicated it here. // Doing it at this point seems to work fine. - RepairDuplicates(); + CheckForDuplicatesAndRepair(); Controls.Add(_reactControl); } ) ); } - private void RepairDuplicates() + private void CheckForDuplicatesAndRepair() { var collectionPath = _model.TheOneEditableCollection.PathToDirectory; // A book's ID may not be changed if we have a TC and the book is actually in the shared folder. // Eventually we may allow it if the book is checked out. - BookInfo.RepairDuplicateInstanceIds( + BookInfo.CheckForDuplicateInstanceIdsAndRepair( collectionPath, (bookPath) => { diff --git a/src/BloomExe/ErrorReporter/HtmlErrorReporter.cs b/src/BloomExe/ErrorReporter/HtmlErrorReporter.cs index e53b0ecbb220..94ed4aba478e 100644 --- a/src/BloomExe/ErrorReporter/HtmlErrorReporter.cs +++ b/src/BloomExe/ErrorReporter/HtmlErrorReporter.cs @@ -4,6 +4,7 @@ using Bloom.Api; using Bloom.MiscUI; using Bloom.ToPalaso; +using Bloom.Utils; using Bloom.web.controllers; using SIL.IO; using SIL.Reporting; @@ -217,6 +218,14 @@ private void NotifyUserOfProblemInternal( try { + string filePath = FileException.GetFilePathIfPresent(exception); + // FileException is a Bloom exception to capture the filepath. We want to report the inner, original exception. + Exception originalException = FileException.UnwrapIfFileException(exception); + if (OneDriveUtils.CheckForAndHandleOneDriveExceptions(originalException, filePath)) + { + return; + } + if (policy == null) { policy = new ShowAlwaysPolicy(); @@ -227,7 +236,7 @@ private void NotifyUserOfProblemInternal( ShowNotifyDialog( ProblemLevel.kNotify, message, - exception, + originalException, reportButtonLabel, onReportButtonClicked, extraButtonLabel, diff --git a/src/BloomExe/MiscUI/StartupScreenManager.cs b/src/BloomExe/MiscUI/StartupScreenManager.cs index deaa55be591b..1e7939af3fce 100644 --- a/src/BloomExe/MiscUI/StartupScreenManager.cs +++ b/src/BloomExe/MiscUI/StartupScreenManager.cs @@ -319,9 +319,14 @@ private static void CloseFastSplashScreen() public static void CloseSplashScreen() { - _splashForm?.FadeAndClose(); //it's going to hang around while it fades, - _doWhenSplashScreenShouldClose?.Invoke(); - DoLastOfAllAfterClosingSplashScreen?.Invoke(); + if (_splashForm == null || !_splashForm.IsDisposed) + { + // We just want to skip the following steps if _splashForm is there but disposed, as this will cause errors. + // They should behave properly if _splashForm is null. + _splashForm?.FadeAndClose(); //it's going to hang around while it fades, + _doWhenSplashScreenShouldClose?.Invoke(); + DoLastOfAllAfterClosingSplashScreen?.Invoke(); + } _splashForm = null; _doWhenSplashScreenShouldClose = null; // Do these actions only once. diff --git a/src/BloomExe/Program.cs b/src/BloomExe/Program.cs index b611d89ee163..172a9311d636 100644 --- a/src/BloomExe/Program.cs +++ b/src/BloomExe/Program.cs @@ -39,6 +39,7 @@ using Microsoft.Web.WebView2.Core; using System.Text; using Bloom.Utils; +using Bloom.web.controllers; namespace Bloom { @@ -1297,6 +1298,22 @@ private static void HandleErrorOpeningProjectWindow(Exception error, string proj _projectContext = null; } + // FileException is a Bloom exception to capture the filepath. We want to report the inner, original exception. + Exception originalError = FileException.UnwrapIfFileException(error); + string errorFilePath = FileException.GetFilePathIfPresent(error); + Logger.WriteError( + $"*** Error loading collection {Path.GetFileNameWithoutExtension(projectPath)}, on filepath: {errorFilePath}", + originalError + ); + + // Normally, NotifyUserOfProblem would take an exception and do this special-exception processing for us. + // But in this case, we don't pass the exception to NotifyUserOfProblem because we may subsequently end up + // calling SendReportWithoutUI. Therefore, we must check for the special exception independently. + if (OneDriveUtils.CheckForAndHandleOneDriveExceptions(error)) + { + return; + } + ErrorResult reportPressedResult = ErrorResult.Yes; // NB: I added the email to this directly because, at least on my machine, the old error report dialog had become unworkable // because, presumably, I haven't set things up properly with gmail. @@ -1322,7 +1339,7 @@ private static void HandleErrorOpeningProjectWindow(Exception error, string proj var additionalPathsToInclude = Directory.GetFiles(dirName); _applicationContainer.ProblemReportApi.SendReportWithoutUI( ProblemLevel.kNonFatal, - error, + originalError, errorMessage, "", additionalPathsToInclude @@ -1333,7 +1350,7 @@ private static void HandleErrorOpeningProjectWindow(Exception error, string proj // No email... just fallback to the WinFormsErrorReporter, which will allow the user to email us. // Unfortunately, we won't be able to automatically get the .bloomCollection file from that. SIL.Reporting.ErrorReport.ReportNonFatalExceptionWithMessage( - error, + originalError, errorMessage ); } diff --git a/src/BloomExe/Utils/FileException.cs b/src/BloomExe/Utils/FileException.cs new file mode 100644 index 000000000000..9cd3fe3c50f7 --- /dev/null +++ b/src/BloomExe/Utils/FileException.cs @@ -0,0 +1,42 @@ +using System; + +namespace Bloom.Utils +{ + /// + /// Attach the filepath to file-specific exceptions, e.g. errors accessing that file + /// + public class FileException : Exception + { + public string FilePath; + + // When WinForms catches exceptions thrown from invoked methods, it only gets the innermost exception + // So we can't put the original exception in innerException or this FilePath will get lost in such cases + // See https://stackoverflow.com/questions/15668334/preserving-exceptions-from-dynamically-invoked-methods + // https://stackoverflow.com/questions/347502/why-does-the-inner-exception-reach-the-threadexception-handler-and-not-the-actua + public Exception OriginalException; + + public FileException(string filePath, Exception originalException) + { + FilePath = filePath; + OriginalException = originalException; + } + + public static string GetFilePathIfPresent(Exception exception) + { + if (exception is FileException) + { + return ((FileException)exception).FilePath; + } + return null; + } + + public static Exception UnwrapIfFileException(Exception exception) + { + if (exception is FileException) + { + return ((FileException)exception).OriginalException; + } + return exception; + } + } +} diff --git a/src/BloomExe/Utils/OneDriveUtils.cs b/src/BloomExe/Utils/OneDriveUtils.cs new file mode 100644 index 000000000000..16e0d5d2e927 --- /dev/null +++ b/src/BloomExe/Utils/OneDriveUtils.cs @@ -0,0 +1,181 @@ +using System; +using System.Linq; +using System.Windows.Forms; +using Bloom.Api; +using Bloom.ErrorReporter; +using Bloom.web.controllers; +using SIL.Reporting; + +namespace Bloom.Utils +{ + public static class OneDriveUtils + { + private static string[] oneDriveErrorCodes = + { + "80070184", // This one is not in Microsoft's list. I got it by disconnecting my internet. + // The rest are from https://support.microsoft.com/en-au/office/what-do-the-onedrive-error-codes-mean-f7a68338-e540-4ebf-ad5d-56c5633acded#ID0EBBH=Error_codes + "80010007", + "80040c81", + "8004de80", + "8004de86", + "8004de85", + "8004de8a", + "8004de90", + "8004de96", + "8004ded7", + "8004dedc", + "8004def0", + "8004def7", + "80070005", + "8007016a", + "80070194", + "80071128", + "80071129" + }; + + public static bool IsOneDriveErrorCode(string errorCode) + { + return oneDriveErrorCodes.Contains(errorCode.ToLowerInvariant()); + } + + public static string GetOneDriveErrorDialogMessage() + { + return L10NSharp.LocalizationManager.GetString( + "ReportProblemDialog.OneDriveErrorMessage", + "There is a problem with your Microsoft OneDrive which is preventing Bloom from accessing files." + ); + } + + public static string GetOneDriveErrorDialogHeader() + { + return L10NSharp.LocalizationManager.GetString( + "ReportProblemDialog.OneDriveProblem", + "OneDrive Problem" + ); + } + + /// + /// We want to show different UI for exceptions caused by the user's OneDrive, which we cannot do anything about + /// And should inform the user about. See BL-12977 + /// Recursively checks inner exceptions. + /// + /// The exception to check. We will check its inner exceptions also. + /// Path of file that we were trying to access, causing this error. To display to the user + /// "fatal" or "nonfatal", for UI display + public static bool CheckForAndHandleOneDriveExceptions( + System.Exception error, + string filePath = "", + string levelOfProblem = "nonfatal" + ) + { + if (error == null) + return false; + + string fileExceptionFilePath = FileException.GetFilePathIfPresent(error); + if (!string.IsNullOrEmpty(fileExceptionFilePath)) + { + filePath = fileExceptionFilePath; + } + // FileException is a Bloom exception to capture the filepath. We want to report the inner, original exception. + Exception originalError = FileException.UnwrapIfFileException(error); + + string errorCode = originalError.HResult.ToString("X"); + + if (!IsOneDriveErrorCode(errorCode)) + { + // This exception is not one of the OneDrive exception codes we check for, + // but check if it has an inner exception that is + if (originalError?.InnerException != null) + { + return CheckForAndHandleOneDriveExceptions( + originalError.InnerException, + filePath + ); + } + else + { + // this is not a special error code, return to normal error handling + return false; + } + } + + Control control = Shell.GetShellOrOtherOpenForm(); + if (control == null) // still possible if we come from a "Details" button + control = FatalExceptionHandler.ControlOnUIThread; + + string logPath = Logger.LogPath.Replace('\\', '/'); + + string errorMessageLabel = "Error Message"; + string errorCodeLabel = "Error Number"; + string filePathLabel = "File"; + string logPathLabel = "Bloom Log"; + void ShowOneDriveExceptionFallbackDialog( + Exception error, + string errorCode, + string filePath, + string logPath + ) + { + // We decided not to localize this for now. + MessageBox.Show( + GetOneDriveErrorDialogMessage() + + Environment.NewLine + + Environment.NewLine + + $"{errorMessageLabel}: {error.Message}{Environment.NewLine}" + + $"{errorCodeLabel}: {errorCode}{Environment.NewLine}" + + $"{filePathLabel}: {filePath}{Environment.NewLine}" + + $"{logPathLabel}: {logPath}", + GetOneDriveErrorDialogHeader(), + MessageBoxButtons.OK, + MessageBoxIcon.Error + ); + } + + if (BloomServer._theOneInstance == null) + { + // We got an error really early, before we can use HTML dialogs + ShowOneDriveExceptionFallbackDialog(originalError, errorCode, filePath, logPath); + + return true; + } + var showOneDriveFallbackDialogAction = new Action(() => + { + ShowOneDriveExceptionFallbackDialog(originalError, errorCode, filePath, logPath); + }); + + string searchOnline = L10NSharp.LocalizationManager.GetString( + "ReportProblemDialog.SearchOnline", + "Search online" + ); + string searchOnlineLink = + errorCode == "80070184" + ? "" // as of now, error "80070184" does not have good online microsoft support results + : $"{searchOnline}
"; + + var reactDialogProps = new + { + level = ProblemLevel.kNotify, // Always use the notify dialog (no report button) for OneDrive errors + message = GetOneDriveErrorDialogMessage(), + detailsBoxText = $"Error message
{originalError.Message}
Error number
" + + $"0x{errorCode} {searchOnlineLink}" + + (!string.IsNullOrEmpty(filePath) ? $"File
{filePath}
" : "") + + $"Bloom Log
", + titleOverride = GetOneDriveErrorDialogHeader(), + titleL10nKeyOverride = "ReportProblemDialog.OneDriveProblem", + themeOverride = levelOfProblem, + }; + + ProblemReportApi.ShowProblemReactDialogWithFallbacks( + showOneDriveFallbackDialogAction, + reactDialogProps, + GetOneDriveErrorDialogHeader(), + null, + control, + originalError, + 450 + ); + + return true; + } + } +} diff --git a/src/BloomExe/WebLibraryIntegration/BookUpload.cs b/src/BloomExe/WebLibraryIntegration/BookUpload.cs index fd7aafb4b109..ed00e40a0eb9 100644 --- a/src/BloomExe/WebLibraryIntegration/BookUpload.cs +++ b/src/BloomExe/WebLibraryIntegration/BookUpload.cs @@ -992,7 +992,7 @@ internal static void BulkRepairInstanceIds( Func okToChangeId ) { - BookInfo.RepairDuplicateInstanceIds(rootFolderPath, okToChangeId); + BookInfo.CheckForDuplicateInstanceIdsAndRepair(rootFolderPath, okToChangeId); } /// diff --git a/src/BloomExe/WebLibraryIntegration/BulkUploader.cs b/src/BloomExe/WebLibraryIntegration/BulkUploader.cs index 5b9b35f84d44..c440c4be5e87 100644 --- a/src/BloomExe/WebLibraryIntegration/BulkUploader.cs +++ b/src/BloomExe/WebLibraryIntegration/BulkUploader.cs @@ -541,7 +541,7 @@ internal static void BulkRepairInstanceIds( Func okToChangeId ) { - BookInfo.RepairDuplicateInstanceIds(rootFolderPath, okToChangeId); + BookInfo.CheckForDuplicateInstanceIdsAndRepair(rootFolderPath, okToChangeId); } } } diff --git a/src/BloomExe/web/controllers/BookCommandsApi.cs b/src/BloomExe/web/controllers/BookCommandsApi.cs index 45913b43452b..45707894e58b 100644 --- a/src/BloomExe/web/controllers/BookCommandsApi.cs +++ b/src/BloomExe/web/controllers/BookCommandsApi.cs @@ -367,6 +367,7 @@ private void HandleExportToWord(Book.Book book) catch (IOException error) { SIL.Reporting.ErrorReport.NotifyUserOfProblem( + error, error.Message, "Could not export the book" ); diff --git a/src/BloomExe/web/controllers/ProblemReportApi.cs b/src/BloomExe/web/controllers/ProblemReportApi.cs index 23c69316eb88..7f0a92ceb913 100644 --- a/src/BloomExe/web/controllers/ProblemReportApi.cs +++ b/src/BloomExe/web/controllers/ProblemReportApi.cs @@ -628,6 +628,99 @@ public static Form GetParentFormForErrorDialogs() // ENHANCE: I think levelOfProblem would benefit from being required and being an enum. + /// + /// Try to show the react problem dialog, but show fallback dialogs if appropriate. + /// Currently called by ShowProblemDialog and OneDriveUtils.CheckForAndHandleOneDriveExceptions, for the logic they share + /// + public static void ShowProblemReactDialogWithFallbacks( + Action showFallbackDialogAction, + dynamic reactDialogProps, + string reactDialogHeader, + IEnumerable additionalPathsToInclude, + Control control, + Exception exception, + int height = 616 + ) + { + SafeInvoke.InvokeIfPossible( + "Show Problem Dialog", + control, + false, + () => + { + // Uses a browser ReactDialog (if possible) to show the problem report + try + { + // We call CloseSplashScreen() above too, where it might help in some cases, but + // this one, while apparently redundant might be wise to keep since closing the splash screen + // needs to be done on the UI thread. + StartupScreenManager.CloseSplashScreen(); + + if (!BloomServer.ServerIsListening) + { + // We can't use the react dialog! + showFallbackDialogAction(); + return; + } + + // Precondition: we must be on the UI thread for Gecko to work. + using ( + var dlg = new ReactDialog( + "problemReportBundle", + reactDialogProps, + reactDialogHeader + ) + ) + { + _additionalPathsToInclude = additionalPathsToInclude; + dlg.FormBorderStyle = FormBorderStyle.FixedToolWindow; // Allows the window to be dragged around + dlg.ControlBox = true; // Add controls like the X button back to the top bar + dlg.Text = ""; // Remove the title from the WinForms top bar + + dlg.Width = 731; + dlg.Height = height; + + // ShowDialog will cause this thread to be blocked (because it spins up a modal) until the dialog is closed. + BloomServer._theOneInstance.RegisterThreadBlocking(); + try + { + // Keep dialog on top of program window if possible. See https://issues.bloomlibrary.org/youtrack/issue/BL-10292. + dlg.ShowDialog(GetParentFormForErrorDialogs()); + } + finally + { + BloomServer._theOneInstance.RegisterThreadUnblocked(); + _additionalPathsToInclude = null; + } + } + } + catch (Exception problemReportException) + { + Logger.WriteError( + "*** ProblemReportApi threw an exception trying to display", + problemReportException + ); + // At this point our problem reporter has failed for some reason, so we want the old WinForms handler + // to report both the original error for which we tried to open our dialog and this new one where + // the dialog itself failed. + // In order to do that, we create a new exception with the original exception (if there was one) as the + // inner exception. We include the message of the exception we just caught. Then we call the + // old WinForms fatal exception report directly. + // In any case, both of the errors will be logged by now. + var message = + "Bloom's error reporting failed: " + problemReportException.Message; + + // Fallback to Winforms in case of trouble getting the browser up + var fallbackReporter = new WinFormsErrorReporter(); + // ENHANCE?: If reporting a non-fatal problem failed, why is the program required to abort? It might be able to handle other tasks successfully + fallbackReporter.ReportFatalException( + new ApplicationException(message, exception ?? problemReportException) + ); + } + } + ); + } + /// /// Shows a problem dialog. /// @@ -685,6 +778,10 @@ public static void ShowProblemDialog( _showingProblemReport = true; } + string filePath = FileException.GetFilePathIfPresent(exception); + // FileException is a Bloom exception to capture the filepath. We want to report the inner, original exception. + Exception originalException = FileException.UnwrapIfFileException(exception); + // We have a better UI for this problem // Note that this will trigger whether it's a plain 'ol System.IO.PathTooLongException, or our own enhanced subclass, Bloom.Utiles.PathTooLongException if (exception is System.IO.PathTooLongException) @@ -693,8 +790,23 @@ public static void ShowProblemDialog( return; } + if ( + OneDriveUtils.CheckForAndHandleOneDriveExceptions( + exception, + filePath, + levelOfProblem + ) + ) + { + lock (_showingProblemReportLock) + { + _showingProblemReport = false; + } + return; + } + GatherReportInfoExceptScreenshot( - exception, + originalException, detailedMessage, shortUserLevelMessage, isShortMessagePreEncoded @@ -711,110 +823,44 @@ public static void ShowProblemDialog( // Now we use SafeInvoke only inside of this extracted method. TryGetScreenshot(controlForScreenshotting); - if (BloomServer._theOneInstance == null) + var showFallbackDialogAction = new Action(() => { - // We got an error really early, before we can use HTML dialogs. Report using the old dialog. - // Hopefully we're still on the one main thread. HtmlErrorReporter.ShowFallbackProblemDialog( levelOfProblem, - exception, + originalException, detailedMessage, shortUserLevelMessage, isShortMessagePreEncoded ); + }); + + if (BloomServer._theOneInstance == null) + { + // We got an error really early, before we can use HTML dialogs. Report using the old dialog. + // Hopefully we're still on the one main thread. + showFallbackDialogAction(); return; } - SafeInvoke.InvokeIfPossible( - "Show Problem Dialog", - controlForScreenshotting, - false, - () => + var reactDialogProps = new { level = levelOfProblem }; + try + { // We want to show the problem dialog with the screenshot if we can, but if we can't, we want to show the fallback dialog. + ShowProblemReactDialogWithFallbacks( + showFallbackDialogAction, + reactDialogProps, + "Problem Report", + additionalPathsToInclude, + controlForScreenshotting, + originalException + ); + } + finally + { + lock (_showingProblemReportLock) { - // Uses a browser ReactDialog (if possible) to show the problem report - try - { - // We call CloseSplashScreen() above too, where it might help in some cases, but - // this one, while apparently redundant might be wise to keep since closing the splash screen - // needs to be done on the UI thread. - StartupScreenManager.CloseSplashScreen(); - - if (!BloomServer.ServerIsListening) - { - // We can't use the react dialog! - HtmlErrorReporter.ShowFallbackProblemDialog( - levelOfProblem, - exception, - detailedMessage, - shortUserLevelMessage, - isShortMessagePreEncoded - ); - return; - } - - // Precondition: we must be on the UI thread for Gecko to work. - using ( - var dlg = new ReactDialog( - "problemReportBundle", - new { level = levelOfProblem }, - "Problem Report" - ) - ) - { - _additionalPathsToInclude = additionalPathsToInclude; - dlg.FormBorderStyle = FormBorderStyle.FixedToolWindow; // Allows the window to be dragged around - dlg.ControlBox = true; // Add controls like the X button back to the top bar - dlg.Text = ""; // Remove the title from the WinForms top bar - - dlg.Width = 731; - dlg.Height = 616; - - // ShowDialog will cause this thread to be blocked (because it spins up a modal) until the dialog is closed. - BloomServer._theOneInstance.RegisterThreadBlocking(); - try - { - // Keep dialog on top of program window if possible. See https://issues.bloomlibrary.org/youtrack/issue/BL-10292. - dlg.ShowDialog(GetParentFormForErrorDialogs()); - } - finally - { - BloomServer._theOneInstance.RegisterThreadUnblocked(); - _additionalPathsToInclude = null; - } - } - } - catch (Exception problemReportException) - { - Logger.WriteError( - "*** ProblemReportApi threw an exception trying to display", - problemReportException - ); - // At this point our problem reporter has failed for some reason, so we want the old WinForms handler - // to report both the original error for which we tried to open our dialog and this new one where - // the dialog itself failed. - // In order to do that, we create a new exception with the original exception (if there was one) as the - // inner exception. We include the message of the exception we just caught. Then we call the - // old WinForms fatal exception report directly. - // In any case, both of the errors will be logged by now. - var message = - "Bloom's error reporting failed: " + problemReportException.Message; - - // Fallback to Winforms in case of trouble getting the browser up - var fallbackReporter = new WinFormsErrorReporter(); - // ENHANCE?: If reporting a non-fatal problem failed, why is the program required to abort? It might be able to handle other tasks successfully - fallbackReporter.ReportFatalException( - new ApplicationException(message, exception ?? problemReportException) - ); - } - finally - { - lock (_showingProblemReportLock) - { - _showingProblemReport = false; - } - } + _showingProblemReport = false; } - ); + } } ///