Skip to content

Commit f6e2fdd

Browse files
annelo-msftannelo-msft
authored
Standardize exceptions thrown from DynamicData (Azure#35605)
* First steps toward changing to InvalidCastExceptions for failed conversions. * Change to more InvalidCastExceptions * pr fb * Move cast exceptions into dynamic layer. * Test cleanup around tests of exceptions. * Test that float over/underflow is treated the same as int over/underflow * Remove dev exception in MutableJsonDocument and change exception thrown for an unsupported format being passed to WriteTo * Ignore float over/underflow tests * pr fb
1 parent 56b24c3 commit f6e2fdd

11 files changed

+262
-303
lines changed

‎eng/Packages.Data.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@
259259
<PackageReference Update="Newtonsoft.Json" Version="10.0.3" />
260260
<PackageReference Update="NSubstitute" Version="3.1.0" />
261261
<PackageReference Update="NUnit" Version="3.13.2" />
262-
<PackageReference Update="NUnit3TestAdapter" Version="4.3.1" />
262+
<PackageReference Update="NUnit3TestAdapter" Version="4.4.2" />
263263
<PackageReference Update="OpenTelemetry.Exporter.InMemory" Version="1.4.0" />
264264
<PackageReference Update="OpenTelemetry.Extensions.AzureMonitor" Version="1.0.0-beta.3" />
265265
<PackageReference Update="OpenTelemetry.Extensions.Hosting" Version="1.4.0" />

‎sdk/core/Azure.Core.Experimental/src/DynamicData.Operators.cs

Lines changed: 127 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Linq;
77
using System.Reflection;
88
using System.Text.Json;
9+
using Azure.Core.Json;
910

1011
namespace Azure.Core.Dynamic
1112
{
@@ -21,66 +22,181 @@ public partial class DynamicData
2122
/// Converts the value to a <see cref="bool"/>.
2223
/// </summary>
2324
/// <param name="value">The value to convert.</param>
24-
public static implicit operator bool(DynamicData value) => value._element.GetBoolean();
25+
public static implicit operator bool(DynamicData value)
26+
{
27+
try
28+
{
29+
return value._element.GetBoolean();
30+
}
31+
catch (InvalidOperationException e)
32+
{
33+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(bool), value._element), e);
34+
}
35+
}
2536

2637
/// <summary>
2738
/// Converts the value to a <see cref="int"/>.
2839
/// </summary>
2940
/// <param name="value">The value to convert.</param>
30-
public static implicit operator int(DynamicData value) => value._element.GetInt32();
41+
public static implicit operator int(DynamicData value)
42+
{
43+
try
44+
{
45+
return value._element.GetInt32();
46+
}
47+
catch (InvalidOperationException e)
48+
{
49+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(int), value._element), e);
50+
}
51+
}
3152

3253
/// <summary>
3354
/// Converts the value to a <see cref="long"/>.
3455
/// </summary>
3556
/// <param name="value">The value to convert.</param>
36-
public static implicit operator long(DynamicData value) => value._element.GetInt64();
57+
public static implicit operator long(DynamicData value)
58+
{
59+
try
60+
{
61+
return value._element.GetInt64();
62+
}
63+
catch (InvalidOperationException e)
64+
{
65+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(long), value._element), e);
66+
}
67+
}
3768

3869
/// <summary>
3970
/// Converts the value to a <see cref="string"/>.
4071
/// </summary>
4172
/// <param name="value">The value to convert.</param>
42-
public static implicit operator string?(DynamicData value) => value._element.GetString();
73+
public static implicit operator string?(DynamicData value)
74+
{
75+
try
76+
{
77+
return value._element.GetString();
78+
}
79+
catch (InvalidOperationException e)
80+
{
81+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(string), value._element), e);
82+
}
83+
}
4384

4485
/// <summary>
4586
/// Converts the value to a <see cref="float"/>.
4687
/// </summary>
4788
/// <param name="value">The value to convert.</param>
48-
public static implicit operator float(DynamicData value) => value._element.GetSingle();
89+
public static implicit operator float(DynamicData value)
90+
{
91+
try
92+
{
93+
return value._element.GetSingle();
94+
}
95+
catch (InvalidOperationException e)
96+
{
97+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(float), value._element), e);
98+
}
99+
}
49100

50101
/// <summary>
51102
/// Converts the value to a <see cref="double"/>.
52103
/// </summary>
53104
/// <param name="value">The value to convert.</param>
54-
public static implicit operator double(DynamicData value) => value._element.GetDouble();
105+
public static implicit operator double(DynamicData value)
106+
{
107+
try
108+
{
109+
return value._element.GetDouble();
110+
}
111+
catch (InvalidOperationException e)
112+
{
113+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(double), value._element), e);
114+
}
115+
}
55116

56117
/// <summary>
57118
/// Converts the value to a <see cref="bool"/> or null.
58119
/// </summary>
59120
/// <param name="value">The value to convert.</param>
60-
public static implicit operator bool?(DynamicData value) => value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetBoolean();
121+
public static implicit operator bool?(DynamicData value)
122+
{
123+
try
124+
{
125+
return value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetBoolean();
126+
}
127+
catch (InvalidOperationException e)
128+
{
129+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(bool?), value._element), e);
130+
}
131+
}
61132

62133
/// <summary>
63134
/// Converts the value to a <see cref="int"/> or null.
64135
/// </summary>
65136
/// <param name="value">The value to convert.</param>
66-
public static implicit operator int?(DynamicData value) => value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetInt32();
137+
public static implicit operator int?(DynamicData value)
138+
{
139+
try
140+
{
141+
return value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetInt32();
142+
}
143+
catch (InvalidOperationException e)
144+
{
145+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(int?), value._element), e);
146+
}
147+
}
67148

68149
/// <summary>
69150
/// Converts the value to a <see cref="long"/> or null.
70151
/// </summary>
71152
/// <param name="value">The value to convert.</param>
72-
public static implicit operator long?(DynamicData value) => value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetInt64();
153+
public static implicit operator long?(DynamicData value)
154+
{
155+
try
156+
{
157+
return value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetInt64();
158+
}
159+
catch (InvalidOperationException e)
160+
{
161+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(long?), value._element), e);
162+
}
163+
}
73164

74165
/// <summary>
75166
/// Converts the value to a <see cref="float"/> or null.
76167
/// </summary>
77168
/// <param name="value">The value to convert.</param>
78-
public static implicit operator float?(DynamicData value) => value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetSingle();
169+
public static implicit operator float?(DynamicData value)
170+
{
171+
try
172+
{
173+
return value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetSingle();
174+
}
175+
catch (InvalidOperationException e)
176+
{
177+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(float?), value._element), e);
178+
}
179+
}
79180

80181
/// <summary>
81182
/// Converts the value to a <see cref="double"/> or null.
82183
/// </summary>
83184
/// <param name="value">The value to convert.</param>
84-
public static implicit operator double?(DynamicData value) => value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetDouble();
185+
public static implicit operator double?(DynamicData value)
186+
{
187+
try
188+
{
189+
return value._element.ValueKind == JsonValueKind.Null ? null : value._element.GetDouble();
190+
}
191+
catch (InvalidOperationException e)
192+
{
193+
throw new InvalidCastException(GetInvalidCastExceptionText(typeof(double?), value._element), e);
194+
}
195+
}
196+
197+
private static string GetInvalidCastExceptionText(Type target, MutableJsonElement element)
198+
{
199+
return $"Unable to cast element to '{target}'. Element has kind '{element.ValueKind}'.";
200+
}
85201
}
86202
}

‎sdk/core/Azure.Core.Experimental/src/DynamicData.cs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ private IEnumerable GetEnumerable()
103103
{
104104
JsonValueKind.Array => new ArrayEnumerator(_element.EnumerateArray(), _options),
105105
JsonValueKind.Object => new ObjectEnumerator(_element.EnumerateObject(), _options),
106-
_ => throw new InvalidOperationException($"Unable to enumerate JSON element."),
106+
_ => throw new InvalidCastException($"Unable to enumerate JSON element of kind {_element.ValueKind}. Cannot cast value to IEnumerable."),
107107
};
108108
}
109109

@@ -163,12 +163,21 @@ private IEnumerable GetEnumerable()
163163

164164
private T ConvertTo<T>()
165165
{
166+
JsonElement element = _element.GetJsonElement();
167+
168+
try
169+
{
166170
#if NET6_0_OR_GREATER
167-
return JsonSerializer.Deserialize<T>(_element.GetJsonElement(), MutableJsonDocument.DefaultJsonSerializerOptions)!;
171+
return JsonSerializer.Deserialize<T>(element, MutableJsonDocument.DefaultJsonSerializerOptions)!;
168172
#else
169-
Utf8JsonReader reader = MutableJsonElement.GetReaderForElement(_element.GetJsonElement());
170-
return JsonSerializer.Deserialize<T>(ref reader, MutableJsonDocument.DefaultJsonSerializerOptions);
173+
Utf8JsonReader reader = MutableJsonElement.GetReaderForElement(element);
174+
return JsonSerializer.Deserialize<T>(ref reader, MutableJsonDocument.DefaultJsonSerializerOptions);
171175
#endif
176+
}
177+
catch (JsonException e)
178+
{
179+
throw new InvalidCastException($"Unable to convert value of kind {element.ValueKind} to type {typeof(T)}.", e);
180+
}
172181
}
173182

174183
/// <inheritdoc/>

‎sdk/core/Azure.Core.Experimental/src/MutableJsonDocument.cs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,15 @@ public MutableJsonElement RootElement
4545
/// <summary>
4646
/// Writes the document to the provided stream as a JSON value.
4747
/// </summary>
48-
/// <param name="stream"></param>
49-
/// <param name="format"></param>
50-
/// <exception cref="ArgumentOutOfRangeException"></exception>
48+
/// <param name="stream">The stream to which to write the document.</param>
49+
/// <param name="format">A format string indicating the format to use when writing the document.</param>
50+
/// <exception cref="FormatException">Thrown if an unsupported value is passed for format.</exception>
51+
/// <remarks>The value of <paramref name="format"/> can be default or 'J' to write the document as JSON.</remarks>
5152
public void WriteTo(Stream stream, StandardFormat format = default)
5253
{
53-
// this is so we can add JSON Patch in the future
54-
if (format != default)
54+
if (format != default || format.Symbol != 'J')
5555
{
56-
throw new ArgumentOutOfRangeException(nameof(format));
56+
throw new FormatException($"Unsupported format {format.Symbol}. Supported formats are: 'J' - JSON.");
5757
}
5858

5959
if (!Changes.HasChanges)
@@ -120,7 +120,7 @@ public void Dispose()
120120
_originalDocument.Dispose();
121121
}
122122

123-
internal MutableJsonDocument(JsonDocument jsonDocument, Memory<byte> utf8Json) : this(jsonDocument.RootElement)
123+
internal MutableJsonDocument(JsonDocument jsonDocument, Memory<byte> utf8Json)
124124
{
125125
_original = utf8Json;
126126
_originalDocument = jsonDocument;
@@ -142,9 +142,6 @@ internal MutableJsonDocument(object? value) : this(value, DefaultJsonSerializerO
142142
/// <param name="type">The type of the value to convert. </param>
143143
internal MutableJsonDocument(object? value, JsonSerializerOptions options, Type? type = null)
144144
{
145-
if (value is JsonDocument)
146-
throw new InvalidOperationException("Calling wrong constructor.");
147-
148145
Type inputType = type ?? (value == null ? typeof(object) : value.GetType());
149146
_original = JsonSerializer.SerializeToUtf8Bytes(value, inputType, options);
150147
_originalDocument = JsonDocument.Parse(_original);

‎sdk/core/Azure.Core.Experimental/tests/DynamicJsonEnumerableTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,13 @@ public void CannotEnumerateNonEnumerable()
147147
}
148148
""");
149149

150-
Assert.Throws<InvalidOperationException>(() =>
150+
Assert.Throws<InvalidCastException>(() =>
151151
{
152152
foreach (dynamic value in json.Foo)
153153
{ }
154154
});
155155

156-
Assert.Throws<InvalidOperationException>(() =>
156+
Assert.Throws<InvalidCastException>(() =>
157157
{
158158
foreach (dynamic value in json.Bar)
159159
{ }

‎sdk/core/Azure.Core.Experimental/tests/DynamicJsonTests.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections;
6+
using System.Text.Json;
67
using Azure.Core.Dynamic;
78
using NUnit.Framework;
89

@@ -839,6 +840,38 @@ public void CanEnumeratePropertiesPascalGetters()
839840
Assert.IsFalse(e.MoveNext());
840841
}
841842

843+
[Test]
844+
public void ThrowsInvalidCastForOriginalJsonValue()
845+
{
846+
dynamic json = BinaryData.FromString(
847+
"""
848+
{
849+
"foo": 1
850+
}
851+
"""
852+
).ToDynamicFromJson(DynamicDataNameMapping.PascalCaseGettersCamelCaseSetters);
853+
854+
Exception e = Assert.Throws<InvalidCastException>(() => { var value = (bool)json.Foo; });
855+
Assert.That(e.Message.Contains(JsonValueKind.Number.ToString()));
856+
}
857+
858+
[Test]
859+
public void ThrowsInvalidCastForChangedJsonValue()
860+
{
861+
dynamic json = BinaryData.FromString(
862+
"""
863+
{
864+
"foo": 1
865+
}
866+
"""
867+
).ToDynamicFromJson(DynamicDataNameMapping.PascalCaseGettersCamelCaseSetters);
868+
869+
json.Foo = true;
870+
871+
Exception e = Assert.Throws<InvalidCastException>(() => { var value = (int)json.Foo; });
872+
Assert.That(e.Message.Contains(JsonValueKind.True.ToString()));
873+
}
874+
842875
#region Helpers
843876
internal static dynamic GetDynamicJson(string json, DynamicDataNameMapping propertyNameCasing = default)
844877
{

0 commit comments

Comments
 (0)