Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix(Revit): Better reporting of critical errors in Revit #2771

Merged
merged 4 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 52 additions & 28 deletions ConnectorRevit/ConnectorRevit/Revit/ErrorEater.cs
Original file line number Diff line number Diff line change
@@ -1,32 +1,28 @@
using System;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;
using Autodesk.Revit.DB;
using Speckle.ConnectorRevit.Entry;
using Speckle.ConnectorRevit.UI;
using Speckle.Core.Kits;
using Speckle.Core.Models;
using Speckle.Core.Logging;

namespace ConnectorRevit.Revit
{
public class ErrorEater : IFailuresPreprocessor
{
private ISpeckleConverter _converter;

private List<Exception> _exceptions = new();
public Dictionary<string, int> CommitErrorsDict = new();
public ErrorEater(ISpeckleConverter converter)
{
_converter = converter;
}

public FailureProcessingResult PreprocessFailures(FailuresAccessor failuresAccessor)
{
IList<FailureMessageAccessor> failList = new List<FailureMessageAccessor>();
var criticalFails = 0;
var resolvedFailures = 0;
var failedElements = new List<ElementId>();
// Inside event handler, get all warnings
failList = failuresAccessor.GetFailureMessages();
var failList = failuresAccessor.GetFailureMessages();
foreach (FailureMessageAccessor failure in failList)
{
// check FailureDefinitionIds against ones that you want to dismiss,
Expand All @@ -35,34 +31,62 @@ public FailureProcessingResult PreprocessFailures(FailuresAccessor failuresAcces
//if (failID == BuiltInFailures.RoomFailures.RoomNotEnclosed)
//{
var t = failure.GetDescriptionText();
_converter.Report.LogConversionError(new Exception(t));

var s = failure.GetSeverity();
if (s == FailureSeverity.Warning) continue;
if (s == FailureSeverity.Warning)
{
// just delete the warnings for now
failuresAccessor.DeleteWarning(failure);
resolvedFailures++;
continue;
}

try
{
failuresAccessor.ResolveFailure(failure);
resolvedFailures++;
}
catch (Exception e)
catch
{
// currently, the whole commit is rolled back. this should be investigated further at a later date
// to properly proceed with commit
failedElements.AddRange(failure.GetFailingElementIds());
//_converter.ConversionErrors.Clear();
_converter.Report.LogConversionError(new Exception(
"Objects failed to bake due to a fatal error!\n" +
"This is likely due to scaling issues - please ensure you've set the correct units on your objects or remove any invalid objects.\n\n" +
"Revit error: " + t));
// logging the error
var exception =
new Speckle.Core.Logging.SpeckleException("Revit commit failed: " + t, e,
level: Sentry.SentryLevel.Warning);
return FailureProcessingResult.ProceedWithCommit;
var idsToDelete = failure.GetFailingElementIds().ToList();

if (failuresAccessor.IsElementsDeletionPermitted(idsToDelete))
{
failuresAccessor.DeleteElements(idsToDelete);
resolvedFailures++;
}
else
{
if (CommitErrorsDict.ContainsKey(t)) CommitErrorsDict[t]++;
else CommitErrorsDict.Add(t, 1);
// currently, the whole commit is rolled back. this should be investigated further at a later date
// to properly proceed with commit
failedElements.AddRange(failure.GetFailingElementIds());

// logging the error
var speckleEx = new SpeckleException($"Fatal Error: {t}");
_exceptions.Add(speckleEx);
SpeckleLog.Logger.Fatal(speckleEx, "Fatal Error: {failureMessage}", t);
}
}
}

failuresAccessor.DeleteAllWarnings();
return FailureProcessingResult.Continue;
if (resolvedFailures > 0) return FailureProcessingResult.ProceedWithCommit;
else return FailureProcessingResult.Continue;
}

public Exception? GetException()
{
if (CommitErrorsDict.Count > 1 && _exceptions.Count > 1)
{
return new AggregateException(_exceptions);
}
else if (CommitErrorsDict.Count == 1 && _exceptions.Count > 0)
{
return _exceptions[0];
}

return null;
}
}
}
26 changes: 22 additions & 4 deletions ConnectorRevit/ConnectorRevit/UI/ConnectorBindingsRevit.Receive.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ await elementTypeMapper.Map(state.Settings.FirstOrDefault(x => x.Slug == "receiv

g.Start();
var failOpts = t.GetFailureHandlingOptions();
failOpts.SetFailuresPreprocessor(new ErrorEater(converter));
var errorEater = new ErrorEater(converter);
failOpts.SetFailuresPreprocessor(errorEater);
failOpts.SetClearAfterRollback(true);
t.SetFailureHandlingOptions(failOpts);
t.Start();
Expand All @@ -113,6 +114,18 @@ await elementTypeMapper.Map(state.Settings.FirstOrDefault(x => x.Slug == "receiv

previousObjects.AddConvertedElements(convertedObjects);
t.Commit();

if (t.GetStatus() == TransactionStatus.RolledBack)
{
var numTotalErrors = errorEater.CommitErrorsDict.Sum(kvp => kvp.Value);
var numUniqueErrors = errorEater.CommitErrorsDict.Keys.Count;

var exception = errorEater.GetException();
if (exception == null) SpeckleLog.Logger.Warning("Revit commit failed with {numUniqueErrors} unique errors and {numTotalErrors} total errors, but the ErrorEater did not capture any exceptions", numUniqueErrors, numTotalErrors);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, including the exception in the log would be useful no?

else SpeckleLog.Logger.Fatal("The Revit API could not resolve {numUniqueErrors} unique errors and {numTotalErrors} total errors when trying to commit the Speckle model. The whole transaction is being rolled back.", numUniqueErrors, numTotalErrors);
return (false, exception ?? new SpeckleException($"The Revit API could not resolve {numUniqueErrors} unique errors and {numTotalErrors} total errors when trying to commit the Speckle model. The whole transaction is being rolled back."));
}

g.Assimilate();
return (true, null);
}
Expand All @@ -132,9 +145,14 @@ await elementTypeMapper.Map(state.Settings.FirstOrDefault(x => x.Slug == "receiv

if (!success)
{
//Don't wrap cancellation token (if it's ours!)
if (exception is OperationCanceledException && progress.CancellationToken.IsCancellationRequested) throw exception;
throw new SpeckleException(exception.Message, exception);
switch (exception)
{
case OperationCanceledException when progress.CancellationToken.IsCancellationRequested:
case AggregateException:
throw exception;
default:
throw new SpeckleException(exception.Message, exception);
}
}

CurrentOperationCancellation = null;
Expand Down
17 changes: 16 additions & 1 deletion Core/Core/Logging/SpeckleLog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ public static ILogger Logger

private static bool _initialized = false;

private static string _logFolderPath;

/// <summary>
/// Initialize logger configuration for a global Serilog.Log logger.
/// </summary>
Expand Down Expand Up @@ -162,8 +164,9 @@ SpeckleLogConfiguration logConfiguration
// if not, disable file sink, even if its enabled in the config
// show a warning about that...
var canLogToFile = true;
_logFolderPath = SpecklePathProvider.LogFolderPath(hostApplicationName, hostApplicationVersion);
var logFilePath = Path.Combine(
SpecklePathProvider.LogFolderPath(hostApplicationName, hostApplicationVersion),
_logFolderPath,
"SpeckleCoreLog.txt"
);
var serilogLogConfiguration = new LoggerConfiguration().MinimumLevel
Expand Down Expand Up @@ -227,6 +230,18 @@ SpeckleLogConfiguration logConfiguration
return logger;
}

public static void OpenCurrentLogFolder()
{
try
{
Process.Start(_logFolderPath);
}
catch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can we ensure we log the exception
image

{
Logger.Error("Unable to open log file folder at the following path, {path}", _logFolderPath);
}
}

private static void _addUserIdToGlobalContextFromDefaultAccount()
{
var machineName = Environment.MachineName;
Expand Down
11 changes: 11 additions & 0 deletions DesktopUI2/DesktopUI2/ViewModels/StreamViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1512,6 +1512,17 @@ public static void HandleCommandException(Exception ex, bool isUserCancel, [Call
Type = NotificationType.Error
};
break;
case AggregateException _:
logLevel = LogEventLevel.Error;
notificationViewModel = new PopUpNotificationViewModel
{
Title = $"😖 {commandPrettyName} Failed!",
Message = "Click to open the log file for a detailed list of error messages",
OnClick = SpeckleLog.OpenCurrentLogFolder,
Type = NotificationType.Error,
Expiration = TimeSpan.FromSeconds(10)
};
break;
case SpeckleException _:
logLevel = LogEventLevel.Error;
notificationViewModel = new PopUpNotificationViewModel
Expand Down