Skip to content

Commit 2fc3f58

Browse files
authored
Catch exceptions when opening files for plugin options serialization (#412)
* Catch exceptions when opening files for plugin options serialization * Cleanup
1 parent b98e308 commit 2fc3f58

File tree

4 files changed

+130
-2
lines changed

4 files changed

+130
-2
lines changed

src/Microsoft.Performance.SDK.Runtime.Tests/Options/StreamPluginOptionsLoaderTests.cs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ public async Task DoNotCloseStreamOnRead_DoesNotCloseStream()
8787
Assert.IsTrue(stream.CanWrite);
8888
}
8989

90+
/// <summary>
91+
/// Asserts that an empty <see cref="Stream"/> returns an empty <see cref="PluginOptionsDto"/>.
92+
/// </summary>
9093
[TestMethod]
9194
public async Task EmptyStream_ReturnsEmptyDto()
9295
{
@@ -99,6 +102,69 @@ public async Task EmptyStream_ReturnsEmptyDto()
99102
Assert.IsTrue(loadedDto.IsEmpty());
100103
}
101104

105+
/// <summary>
106+
/// Asserts that <see cref="StreamPluginOptionsLoader{T}.TryLoadAsync"/> returns <c>null</c> if an exception is thrown
107+
/// when getting the <see cref="Stream"/> to load from.
108+
/// </summary>
109+
[TestMethod]
110+
public async Task GetStreamThrows_ReturnsNull()
111+
{
112+
var sut = new ThrowingStreamPluginOptionsLoader(true, false);
113+
114+
var loadedDto = await sut.TryLoadAsync();
115+
116+
Assert.IsNull(loadedDto);
117+
}
118+
119+
/// <summary>
120+
/// Asserts that <see cref="StreamPluginOptionsLoader{T}.TryLoadAsync"/> returns <c>null</c> if an exception is thrown
121+
/// when checking if the <see cref="Stream"/> to load from has content.
122+
/// </summary>
123+
[TestMethod]
124+
public async Task HasContentThrows_ReturnsNull()
125+
{
126+
using MemoryStream stream = new MemoryStream();
127+
var sut = new ThrowingStreamPluginOptionsLoader(false, true);
128+
129+
var loadedDto = await sut.TryLoadAsync();
130+
131+
Assert.IsNull(loadedDto);
132+
}
133+
134+
private sealed class ThrowingStreamPluginOptionsLoader
135+
: StreamPluginOptionsLoader<MemoryStream>
136+
{
137+
private readonly bool throwInGetStream;
138+
private readonly bool throwInHasContent;
139+
140+
public ThrowingStreamPluginOptionsLoader(bool throwInGetStream, bool throwInHasContent)
141+
: base(true, Logger.Null)
142+
{
143+
this.throwInGetStream = throwInGetStream;
144+
this.throwInHasContent = throwInHasContent;
145+
}
146+
147+
protected override MemoryStream GetStream()
148+
{
149+
if (this.throwInGetStream)
150+
{
151+
throw new InvalidOperationException();
152+
}
153+
154+
return new MemoryStream();
155+
}
156+
157+
protected override bool HasContent(MemoryStream stream)
158+
{
159+
if (this.throwInHasContent)
160+
{
161+
throw new InvalidOperationException();
162+
}
163+
164+
return true;
165+
}
166+
}
167+
102168
private sealed class TestStreamPluginOptionsLoader
103169
: StreamPluginOptionsLoader<MemoryStream>
104170
{

src/Microsoft.Performance.SDK.Runtime.Tests/Options/StreamPluginOptionsSaverTests.cs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,42 @@ public async Task DoNotCloseStreamOnWrite_DoesNotCloseStream()
8282
Assert.IsTrue(stream.CanWrite);
8383
}
8484

85+
/// <summary>
86+
/// Asserts that <see cref="StreamPluginOptionsSaver.TrySaveAsync"/> returns <c>false</c> if an exception is thrown
87+
/// when getting the <see cref="Stream"/> to save to.
88+
/// </summary>
89+
[TestMethod]
90+
public async Task GetStreamThrows_ReturnsFalse()
91+
{
92+
var sut = new ThrowingStreamPluginOptionsSaver(true);
93+
94+
var saved = await sut.TrySaveAsync(TestPluginOptionDto.PluginOptionsDto());
95+
96+
Assert.IsFalse(saved);
97+
}
98+
99+
private sealed class ThrowingStreamPluginOptionsSaver
100+
: StreamPluginOptionsSaver
101+
{
102+
private readonly bool throwOnGetStream;
103+
104+
public ThrowingStreamPluginOptionsSaver(bool throwOnGetStream)
105+
: base(true, Logger.Null)
106+
{
107+
this.throwOnGetStream = throwOnGetStream;
108+
}
109+
110+
protected override Stream GetStream()
111+
{
112+
if (this.throwOnGetStream)
113+
{
114+
throw new InvalidOperationException();
115+
}
116+
117+
return new MemoryStream();
118+
}
119+
}
120+
85121
private sealed class TestPluginOptionsSaver
86122
: StreamPluginOptionsSaver
87123
{

src/Microsoft.Performance.SDK.Runtime/Options/Serialization/Loading/StreamPluginOptionsLoader.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,17 @@ public async Task<PluginOptionsDto> TryLoadAsync()
4646
{
4747
var jsonSerializerOptions = PluginOptionsDtoJsonSerialization.GetJsonSerializerOptions();
4848

49-
var stream = GetStream();
49+
T stream;
50+
try
51+
{
52+
stream = GetStream();
53+
}
54+
catch (Exception e)
55+
{
56+
this.logger?.Error(e, "Failed to get stream to load plugin options from.");
57+
return null;
58+
}
59+
5060
try
5161
{
5262
if (!HasContent(stream))
@@ -77,6 +87,8 @@ private protected virtual string GetDeserializeErrorMessage(Exception exception)
7787

7888
/// <summary>
7989
/// Gets the <see cref="Stream"/> from which to load the <see cref="PluginOptionsDto"/>.
90+
/// If this method throws an exception, the call to <see cref="TryLoadAsync"/> will be considered
91+
/// as cleanly failed and return <c>null</c>.
8092
/// </summary>
8193
/// <returns>
8294
/// The <see cref="Stream"/> from which to load the <see cref="PluginOptionsDto"/>.
@@ -85,6 +97,8 @@ private protected virtual string GetDeserializeErrorMessage(Exception exception)
8597

8698
/// <summary>
8799
/// Determines whether <paramref name="stream"/> has content.
100+
/// If this method throws an exception, the call to <see cref="TryLoadAsync"/> will be considered
101+
/// as cleanly failed and return <c>null</c>.
88102
/// </summary>
89103
/// <param name="stream">
90104
/// The <see cref="Stream"/> of type <typeparamref name="T"/> to check.

src/Microsoft.Performance.SDK.Runtime/Options/Serialization/Saving/StreamPluginOptionsSaver.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,17 @@ public async Task<bool> TrySaveAsync(PluginOptionsDto optionsDto)
4040
{
4141
var jsonSerializerOptions = PluginOptionsDtoJsonSerialization.GetJsonSerializerOptions();
4242

43-
var stream = GetStream();
43+
Stream stream;
44+
try
45+
{
46+
stream = GetStream();
47+
}
48+
catch (Exception e)
49+
{
50+
this.logger?.Error(e, "Failed to get stream to save plugin options to.");
51+
return false;
52+
}
53+
4454
try
4555
{
4656
await JsonSerializer.SerializeAsync(stream, optionsDto, jsonSerializerOptions);
@@ -67,6 +77,8 @@ private protected virtual string GetSerializeErrorMessage(Exception exception)
6777

6878
/// <summary>
6979
/// Gets the <see cref="Stream"/> to save to.
80+
/// If this method throws an exception, the call to <see cref="TrySaveAsync"/> will be considered
81+
/// as cleanly failed and return <c>false</c>.
7082
/// </summary>
7183
/// <returns>
7284
/// The <see cref="Stream"/> to save to.

0 commit comments

Comments
 (0)