Skip to content

Commit

Permalink
[nnyeah] Centralize error handling (#15064)
Browse files Browse the repository at this point in the history
- Due to microsoft/vstest#3658 it is not possible to test code that exits the process on error
- Create a base class for nnyeah exceptions that we want to explicitly report (and not crash), ConversionException
- Move Main to Main2 and wrap it in a try/catch for this exception
  • Loading branch information
chamons authored May 23, 2022
1 parent da7dd02 commit 7e8bd5c
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 28 deletions.
6 changes: 6 additions & 0 deletions tools/nnyeah/nnyeah/Errors.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions tools/nnyeah/nnyeah/Errors.resx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
<value>If you want to compare assemblies, the earlier and later assembly options *must* be set.</value>
</data>

<data name="N0008" xml:space="preserve">
<value>Unexpected error - Please file a bug report at https://github.com/xamarin/xamarin-macios/issues/new</value>
</data>

<data name="E0001" xml:space="preserve">
<value>Input file '{0}' doesn't exist.</value>
</data>
Expand Down
2 changes: 1 addition & 1 deletion tools/nnyeah/nnyeah/MemberNotFoundException.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using System;
namespace Microsoft.MaciOS.Nnyeah {
public class MemberNotFoundException : Exception {
public class MemberNotFoundException : ConversionException {
public MemberNotFoundException (string memberName)
: base ($"Member {memberName} not found.")
{
Expand Down
66 changes: 40 additions & 26 deletions tools/nnyeah/nnyeah/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,34 @@
using System.Diagnostics.CodeAnalysis;

namespace Microsoft.MaciOS.Nnyeah {
public class ConversionException : Exception {
public ConversionException (string message) : base (message)
{
}

public ConversionException (string message, params object? [] args) : base (string.Format(message, args))
{
}
}

public class Program {
static void Main (string [] args)
static int Main (string [] args)
{
try {
Main2 (args);
return 0;
}
catch (ConversionException e) {
Console.Error.WriteLine (e.Message);
return 1;
} catch (Exception e) {
Console.Error.WriteLine (Errors.N0008);
Console.Error.WriteLine (e);
return 1;
}
}

static void Main2 (string [] args)
{
var doHelp = false;
string? infile = null, outfile = null;
Expand All @@ -35,22 +61,20 @@ static void Main (string [] args)
}

if (infile is null || outfile is null) {
Console.Error.WriteLine (Errors.E0014);
Environment.Exit (1);
throw new ConversionException (Errors.E0014);
}

// TODO - Long term this should default to files packaged within the tool but allow overrides
if (xamarinAssembly is null || microsoftAssembly is null) {
Console.Error.WriteLine (Errors.E0015);
Environment.Exit (1);
throw new ConversionException (Errors.E0015);
}

if (doHelp) {
PrintOptions (options, Console.Out);
Environment.Exit (0);
}
ProcessAssembly (xamarinAssembly!, microsoftAssembly!, infile!,
outfile!, verbose, forceOverwrite, suppressWarnings);
else {
ProcessAssembly (xamarinAssembly!, microsoftAssembly!, infile!, outfile!, verbose, forceOverwrite, suppressWarnings);
}
}

public static void ProcessAssembly (string xamarinAssembly,
Expand Down Expand Up @@ -134,9 +158,7 @@ static bool TryLoadTypeAndModuleMap (string earlier, string later, bool publicOn

return Reworker.CreateReworker (stm, moduleContainer, typeMap);
} catch (Exception e) {
Console.Error.WriteLine (Errors.E0003, infile, e.Message);
Environment.Exit (1);
throw;
throw new ConversionException (Errors.E0003, infile, e.Message);
}
}

Expand All @@ -149,13 +171,11 @@ static void ReworkFile (string infile, string outfile, bool verbose, bool forceO
var transforms = new List<string> ();

if (!File.Exists (infile)) {
Console.Error.WriteLine (Errors.E0001, infile);
Environment.Exit (1);
throw new ConversionException (Errors.E0001, infile);
}

if (File.Exists (outfile) && !forceOverwrite) {
Console.Error.WriteLine (Errors.E0002, outfile);
Environment.Exit (1);
throw new ConversionException (Errors.E0002, outfile);
}

if (CreateReworker (infile, typeMap, xamarinModule, microsoftModule) is Reworker reworker) {
Expand All @@ -172,14 +192,11 @@ static void ReworkFile (string infile, string outfile, bool verbose, bool forceO
warnings.ForEach (Console.WriteLine);
}
} catch (TypeNotFoundException e) {
Console.Error.Write (Errors.E0012, e.TypeName);
Environment.Exit (1);
throw new ConversionException (Errors.E0012, e.TypeName);
} catch (MemberNotFoundException e) {
Console.Error.WriteLine (Errors.E0013, e.MemberName);
Environment.Exit (1);
throw new ConversionException (Errors.E0013, e.MemberName);
} catch (Exception e) {
Console.Error.WriteLine (Errors.E0004, e.Message);
Environment.Exit (1);
throw new ConversionException (Errors.E0004, e.Message);
}
} else {
if (verbose) {
Expand All @@ -193,15 +210,12 @@ static Stream TryOpenRead (string path)
try {
var stm = new FileStream (path, FileMode.Open, FileAccess.Read);
if (stm is null) {
Console.Error.WriteLine (Errors.E0006, path, "");
Environment.Exit (1);
throw new ConversionException (Errors.E0006, path, "");
}
return stm;
} catch (Exception e) {
Console.Error.WriteLine (Errors.E0006, path, e.Message);
Environment.Exit (1);
throw new ConversionException (Errors.E0006, path, e.Message);
}
return new MemoryStream (); // never reached, thanks code analysis
}

static void PrintOptions (OptionSet options, TextWriter writer)
Expand Down
2 changes: 1 addition & 1 deletion tools/nnyeah/nnyeah/TypeNotFoundException.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using System;
namespace Microsoft.MaciOS.Nnyeah {
public class TypeNotFoundException : Exception {
public class TypeNotFoundException : ConversionException {
public TypeNotFoundException (string typeName)
: base ($"The type {typeName} was not found.")
{
Expand Down

5 comments on commit 7e8bd5c

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

💻 [CI Build] Tests on macOS Mac Catalina (10.15) passed 💻

All tests on macOS Mac Catalina (10.15) passed.

Pipeline on Agent
Hash: 7e8bd5cac42a954b21a43c02f6efc937eec625c4

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

❌ [CI Build] Tests on macOS M1 - Mac Big Sur (11.5) failed ❌

Failed tests are:

  • xammac_tests
  • monotouch-test

Pipeline on Agent
Hash: 7e8bd5cac42a954b21a43c02f6efc937eec625c4

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

📋 [CI Build] API Diff 📋

API diff (for current PR)

ℹ️ API Diff (from PR only) (please review changes)

API diff: vsdrops gist

Xamarin
.NET
Xamarin vs .NET
iOS vs Mac Catalyst (.NET)

API diff (vs stable)

✅ API Diff from stable

API diff: vsdrops gist

Xamarin
.NET
Xamarin vs .NET
iOS vs Mac Catalyst (.NET)

Generator diff

Generator Diff (no change)

Pipeline on Agent XAMMINI-068.Monterey'
Hash: 7e8bd5cac42a954b21a43c02f6efc937eec625c4

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

📚 [CI Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMMINI-049.Monterey
Hash: 7e8bd5cac42a954b21a43c02f6efc937eec625c4

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌

Tests failed on VSTS: simulator tests iOS.

Test results

3 tests failed, 1028 tests' device not found, 225 tests passed.

Failed tests

  • monotouch-test/watchOS 32-bits - simulator/Debug (LinkSdk): TimedOut
  • monotouch-test/watchOS 32-bits - simulator/Debug (static registrar): Crashed
  • monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): Crashed

Pipeline on Agent XAMBOT-1044.Monterey'
[nnyeah] Centralize error handling (#15064)

Please sign in to comment.