Skip to content

Commit

Permalink
Improvements to Scripting API exception handling (dotnet#11416)
Browse files Browse the repository at this point in the history
  • Loading branch information
tmat committed May 20, 2016
1 parent aad4fe5 commit 12e5e17
Show file tree
Hide file tree
Showing 10 changed files with 580 additions and 137 deletions.
115 changes: 58 additions & 57 deletions src/Interactive/Features/Interactive/Core/InteractiveHost.Service.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,18 @@ private struct EvaluationState
internal readonly ScriptOptions ScriptOptions;

internal EvaluationState(
ScriptState<object> scriptState,
ScriptState<object> scriptStateOpt,
ScriptOptions scriptOptions,
ImmutableArray<string> sourceSearchPaths,
ImmutableArray<string> referenceSearchPaths,
string workingDirectory)
{
ScriptStateOpt = scriptState;
Debug.Assert(scriptOptions != null);
Debug.Assert(!sourceSearchPaths.IsDefault);
Debug.Assert(!referenceSearchPaths.IsDefault);
Debug.Assert(workingDirectory != null);

ScriptStateOpt = scriptStateOpt;
ScriptOptions = scriptOptions;
SourceSearchPaths = sourceSearchPaths;
ReferenceSearchPaths = referenceSearchPaths;
Expand All @@ -74,6 +79,8 @@ internal EvaluationState(

internal EvaluationState WithScriptState(ScriptState<object> state)
{
Debug.Assert(state != null);

return new EvaluationState(
state,
ScriptOptions,
Expand All @@ -84,6 +91,8 @@ internal EvaluationState WithScriptState(ScriptState<object> state)

internal EvaluationState WithOptions(ScriptOptions options)
{
Debug.Assert(options != null);

return new EvaluationState(
ScriptStateOpt,
options,
Expand All @@ -104,7 +113,7 @@ internal EvaluationState WithOptions(ScriptOptions options)
public Service()
{
var initialState = new EvaluationState(
scriptState: null,
scriptStateOpt: null,
scriptOptions: ScriptOptions.Default,
sourceSearchPaths: ImmutableArray<string>.Empty,
referenceSearchPaths: ImmutableArray<string>.Empty,
Expand Down Expand Up @@ -442,12 +451,8 @@ private async Task<EvaluationState> ExecuteAsync(Task<EvaluationState> lastTask,
// remove references and imports from the options, they have been applied and will be inherited from now on:
state = state.WithOptions(state.ScriptOptions.RemoveImportsAndReferences());

var newScriptState = await ExecuteOnUIThread(script, state.ScriptStateOpt).ConfigureAwait(false);
if (newScriptState != null)
{
DisplaySubmissionResult(newScriptState);
state = state.WithScriptState(newScriptState);
}
var newScriptState = await ExecuteOnUIThread(script, state.ScriptStateOpt, displayResult: true).ConfigureAwait(false);
state = state.WithScriptState(newScriptState);
}
}
catch (Exception e)
Expand All @@ -462,11 +467,15 @@ private async Task<EvaluationState> ExecuteAsync(Task<EvaluationState> lastTask,
return state;
}

private void DisplaySubmissionResult(ScriptState<object> state)
private void DisplayException(Exception e)
{
if (state.Script.HasReturnValue())
if (e is FileLoadException && e.InnerException is InteractiveAssemblyLoaderException)
{
_globals.Print(state.ReturnValue);
Console.Error.WriteLine(e.InnerException.Message);
}
else
{
Console.Error.Write(_replServiceProvider.ObjectFormatter.FormatException(e));
}
}

Expand Down Expand Up @@ -633,7 +642,7 @@ private async Task<EvaluationState> InitializeContextAsync(

if (scriptPathOpt != null)
{
var newScriptState = await ExecuteFileAsync(rspState, scriptPathOpt).ConfigureAwait(false);
var newScriptState = await TryExecuteFileAsync(rspState, scriptPathOpt).ConfigureAwait(false);
if (newScriptState != null)
{
// remove references and imports from the options, they have been applied and will be inherited from now on:
Expand Down Expand Up @@ -725,61 +734,53 @@ private async Task<EvaluationState> ExecuteFileAsync(
string path)
{
var state = await ReportUnhandledExceptionIfAny(lastTask).ConfigureAwait(false);
var success = false;
try
string fullPath = ResolveRelativePath(path, state.WorkingDirectory, state.SourceSearchPaths, displayPath: false);
if (fullPath != null)
{
var fullPath = ResolveRelativePath(path, state.WorkingDirectory, state.SourceSearchPaths, displayPath: false);

var newScriptState = await ExecuteFileAsync(state, fullPath).ConfigureAwait(false);
var newScriptState = await TryExecuteFileAsync(state, fullPath).ConfigureAwait(false);
if (newScriptState != null)
{
success = true;
state = state.WithScriptState(newScriptState);
return CompleteExecution(state.WithScriptState(newScriptState), operation, success: newScriptState.Exception == null);
}
}
finally
{
state = CompleteExecution(state, operation, success);
}

return state;
return CompleteExecution(state, operation, success: false);
}

/// <summary>
/// Executes specified script file as a submission.
/// </summary>
/// <returns>True if the code has been executed. False if the code doesn't compile.</returns>
/// <remarks>
/// All errors are written to the error output stream.
/// Uses source search paths to resolve unrooted paths.
/// </remarks>
private async Task<ScriptState<object>> ExecuteFileAsync(EvaluationState state, string fullPath)
private async Task<ScriptState<object>> TryExecuteFileAsync(EvaluationState state, string fullPath)
{
Debug.Assert(PathUtilities.IsAbsolute(fullPath));

string content = null;
if (fullPath != null)
try
{
Debug.Assert(PathUtilities.IsAbsolute(fullPath));
try
{
content = File.ReadAllText(fullPath);
}
catch (Exception e)
using (var reader = File.OpenText(fullPath))
{
Console.Error.WriteLine(e.Message);
content = await reader.ReadToEndAsync().ConfigureAwait(false);
}
}
catch (Exception e)
{
// file read errors:
Console.Error.WriteLine(e.Message);
return null;
}

ScriptState<object> newScriptState = null;
if (content != null)
Script<object> script = TryCompile(state.ScriptStateOpt?.Script, content, fullPath, state.ScriptOptions);
if (script == null)
{
Script<object> script = TryCompile(state.ScriptStateOpt?.Script, content, fullPath, state.ScriptOptions);
if (script != null)
{
newScriptState = await ExecuteOnUIThread(script, state.ScriptStateOpt).ConfigureAwait(false);
}
// compilation errors:
return null;
}

return newScriptState;
return await ExecuteOnUIThread(script, state.ScriptStateOpt, displayResult: false).ConfigureAwait(false);
}

private static void DisplaySearchPaths(TextWriter writer, List<string> attemptedFilePaths)
Expand All @@ -801,28 +802,28 @@ private static void DisplaySearchPaths(TextWriter writer, List<string> attempted
}
}

private async Task<ScriptState<object>> ExecuteOnUIThread(Script<object> script, ScriptState<object> stateOpt)
private async Task<ScriptState<object>> ExecuteOnUIThread(Script<object> script, ScriptState<object> stateOpt, bool displayResult)
{
return await ((Task<ScriptState<object>>)s_control.Invoke(
(Func<Task<ScriptState<object>>>)(async () =>
{
try
{
var task = (stateOpt == null) ?
script.RunAsync(_globals, CancellationToken.None) :
script.RunFromAsync(stateOpt, CancellationToken.None);
return await task.ConfigureAwait(false);
}
catch (FileLoadException e) when (e.InnerException is InteractiveAssemblyLoaderException)
var task = (stateOpt == null) ?
script.RunAsync(_globals, catchException: e => true, cancellationToken: CancellationToken.None) :
script.RunFromAsync(stateOpt, catchException: e => true, cancellationToken: CancellationToken.None);

var newState = await task.ConfigureAwait(false);

if (newState.Exception != null)
{
Console.Error.WriteLine(e.InnerException.Message);
return null;
DisplayException(newState.Exception);
}
catch (Exception e)
else if (displayResult && newState.Script.HasReturnValue())
{
Console.Error.Write(_replServiceProvider.ObjectFormatter.FormatException(e));
return null;
_globals.Print(newState.ReturnValue);
}

return newState;

}))).ConfigureAwait(false);
}

Expand Down
22 changes: 18 additions & 4 deletions src/Interactive/HostTest/InteractiveHostTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
Expand Down Expand Up @@ -366,11 +367,10 @@ public void AsyncExecuteFile_SourceKind()
}

[Fact]
public void AsyncExecuteFile_NonExistingFile()
public async Task AsyncExecuteFile_NonExistingFile()
{
var task = _host.ExecuteFileAsync("non existing file");
task.Wait();
Assert.False(task.Result.Success);
var result = await _host.ExecuteFileAsync("non existing file");
Assert.False(result.Success);

var errorOut = ReadErrorOutputToEnd().Trim();
Assert.Contains(FeaturesResources.SpecifiedFileNotFound, errorOut, StringComparison.Ordinal);
Expand Down Expand Up @@ -1155,6 +1155,20 @@ public void Exception()
Assert.True(error.StartsWith(new Exception().Message));
}

[Fact, WorkItem(10883, "https://github.com/dotnet/roslyn/issues/10883")]
public void PreservingDeclarationsOnException()
{
Execute(@"int i = 100;");
Execute(@"int j = 20; throw new System.Exception(""Bang!""); int k = 3;");
Execute(@"i + j + k");

var output = ReadOutputToEnd();
var error = ReadErrorOutputToEnd();

AssertEx.AssertEqualToleratingWhitespaceDifferences("120", output);
AssertEx.AssertEqualToleratingWhitespaceDifferences("Bang!", error);
}

#region Submission result printing - null/void/value.

[Fact]
Expand Down
30 changes: 30 additions & 0 deletions src/Scripting/CSharpTest/CommandLineRunnerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -820,5 +820,35 @@ public class Lib2
«Gray»
> ", runner.Console.Out.ToString());
}

[Fact]
[WorkItem(6580, "https://github.com/dotnet/roslyn/issues/6580")]
public void PreservingDeclarationsOnException()
{
var runner = CreateRunner(input:
@"int i = 100;
int j = 20; throw new System.Exception(""Bang!""); int k = 3;
i + j + k
");
runner.RunInteractive();

AssertEx.AssertEqualToleratingWhitespaceDifferences(
$@"Microsoft (R) Visual C# Interactive Compiler version {s_compilerVersion}
Copyright (C) Microsoft Corporation. All rights reserved.
Type ""#help"" for more information.
> int i = 100;
> int j = 20; throw new System.Exception(""Bang!""); int k = 3;
«Red»
Bang!
«Gray»
> i + j + k
120
> ", runner.Console.Out.ToString());

AssertEx.AssertEqualToleratingWhitespaceDifferences(
@"Bang!",
runner.Console.Error.ToString());
}
}
}
Loading

0 comments on commit 12e5e17

Please sign in to comment.