Skip to content

Commit 19e322f

Browse files
authored
Handle unexpected exception due to column type change in datasource (#2681)
## Why make this change? When an API owner starts up DAB, the GraphQL data types for the fields are automatically derived from the SQL column types. However, if the API owner later changes the column type without restarting DAB, the requests will fail at runtime due to failure with parsing the JSON result in the leaf node resolver. Due to the unhandled exception, this will result in an error message `One of the identified items was in an invalid format` or `"The requested operation requires an element of type 'Number', but the target element has type 'String'.` ## What is this change? 1. Wrap the field value parsing to catch specific exception types and throw a clearer exception 2. Ensure location and path in errors are retained for DataApiBuilderException for 4xx status codes ## How was this tested? - [x] Manual test - [] Integration Tests - [] Unit Tests ## Sample Request(s) Step 1: Create sample table in datasource ``` -- Create the table CREATE TABLE Employees ( EmployeeID INT PRIMARY KEY, Age INT NOT NULL, Salary INT NOT NULL ); -- Insert sample data INSERT INTO Employees (EmployeeID, Age, Salary) VALUES (1, 25, 50000), (2, 30, 60000), (3, 35, 70000), (4, 40, 80000), (5, 45, 90000); ``` Step 2: Configure dab config with following entity ``` "Employees": { "source": { "object": "dbo.Employees", "type": "table" }, "permissions": [ { "role": "anonymous", "actions": [ { "action": "*" } ] } ] }, ``` Verify employees query runs fine with Salary field ``` { employees { items { EmployeeID Salary } } } ``` Step 3: Update Salary column type ``` ALTER TABLE Employees ALTER COLUMN Salary NVARCHAR(100); ``` Run employees query again, response returned ``` { "errors": [ { "message": "The value could not be parsed for configured GraphQL data type Int", "locations": [ { "line": 5, "column": 13 } ], "path": [ "employees", "items", 0, "Salary" ], "extensions": { "code": "GraphQLMapping" } }, ... ] } ```
1 parent 4c499ed commit 19e322f

10 files changed

+72
-30
lines changed
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System.Net;
5+
6+
namespace Azure.DataApiBuilder.Config.Utilities;
7+
8+
public static class HttpStatusCodeExtensions
9+
{
10+
/// <summary>
11+
/// Check for status code within 4xx range
12+
/// </summary>
13+
/// <param name="statusCode"></param>
14+
/// <returns></returns>
15+
public static bool IsClientError(this HttpStatusCode statusCode)
16+
{
17+
int code = (int)statusCode;
18+
return code >= 400 && code < 500;
19+
}
20+
}

src/Core/Services/ExecutionHelper.cs

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22
// Licensed under the MIT License.
33

44
using System.Globalization;
5+
using System.Net;
56
using System.Text.Json;
67
using Azure.DataApiBuilder.Config.ObjectModel;
78
using Azure.DataApiBuilder.Core.Configurations;
89
using Azure.DataApiBuilder.Core.Models;
910
using Azure.DataApiBuilder.Core.Resolvers;
1011
using Azure.DataApiBuilder.Core.Resolvers.Factories;
12+
using Azure.DataApiBuilder.Service.Exceptions;
1113
using Azure.DataApiBuilder.Service.GraphQLBuilder;
1214
using Azure.DataApiBuilder.Service.GraphQLBuilder.CustomScalars;
1315
using Azure.DataApiBuilder.Service.GraphQLBuilder.GraphQLTypes;
@@ -155,27 +157,41 @@ fieldValue.ValueKind is not (JsonValueKind.Undefined or JsonValueKind.Null))
155157
// transform it into the runtime type.
156158
// We also want to ensure here that we do not unnecessarily convert values to
157159
// strings and then force the conversion to parse them.
158-
return namedType switch
160+
try
159161
{
160-
StringType => fieldValue.GetString(), // spec
161-
ByteType => fieldValue.GetByte(),
162-
ShortType => fieldValue.GetInt16(),
163-
IntType => fieldValue.GetInt32(), // spec
164-
LongType => fieldValue.GetInt64(),
165-
FloatType => fieldValue.GetDouble(), // spec
166-
SingleType => fieldValue.GetSingle(),
167-
DecimalType => fieldValue.GetDecimal(),
168-
DateTimeType => DateTimeOffset.TryParse(fieldValue.GetString()!, DateTimeFormatInfo.InvariantInfo, DateTimeStyles.AssumeUniversal, out DateTimeOffset date) ? date : null, // for DW when datetime is null it will be in "" (double quotes) due to stringagg parsing and hence we need to ensure parsing is correct.
169-
DateType => DateTimeOffset.TryParse(fieldValue.GetString()!, out DateTimeOffset date) ? date : null,
170-
LocalTimeType => fieldValue.GetString()!.Equals("null", StringComparison.OrdinalIgnoreCase) ? null : LocalTimePattern.ExtendedIso.Parse(fieldValue.GetString()!).Value,
171-
ByteArrayType => fieldValue.GetBytesFromBase64(),
172-
BooleanType => fieldValue.GetBoolean(), // spec
173-
UrlType => new Uri(fieldValue.GetString()!),
174-
UuidType => fieldValue.GetGuid(),
175-
TimeSpanType => TimeSpan.Parse(fieldValue.GetString()!),
176-
AnyType => fieldValue.ToString(),
177-
_ => fieldValue.GetString()
178-
};
162+
return namedType switch
163+
{
164+
StringType => fieldValue.GetString(), // spec
165+
ByteType => fieldValue.GetByte(),
166+
ShortType => fieldValue.GetInt16(),
167+
IntType => fieldValue.GetInt32(), // spec
168+
LongType => fieldValue.GetInt64(),
169+
FloatType => fieldValue.GetDouble(), // spec
170+
SingleType => fieldValue.GetSingle(),
171+
DecimalType => fieldValue.GetDecimal(),
172+
DateTimeType => DateTimeOffset.TryParse(fieldValue.GetString()!, DateTimeFormatInfo.InvariantInfo, DateTimeStyles.AssumeUniversal, out DateTimeOffset date) ? date : null, // for DW when datetime is null it will be in "" (double quotes) due to stringagg parsing and hence we need to ensure parsing is correct.
173+
DateType => DateTimeOffset.TryParse(fieldValue.GetString()!, out DateTimeOffset date) ? date : null,
174+
LocalTimeType => fieldValue.GetString()!.Equals("null", StringComparison.OrdinalIgnoreCase) ? null : LocalTimePattern.ExtendedIso.Parse(fieldValue.GetString()!).Value,
175+
ByteArrayType => fieldValue.GetBytesFromBase64(),
176+
BooleanType => fieldValue.GetBoolean(), // spec
177+
UrlType => new Uri(fieldValue.GetString()!),
178+
UuidType => fieldValue.GetGuid(),
179+
TimeSpanType => TimeSpan.Parse(fieldValue.GetString()!),
180+
AnyType => fieldValue.ToString(),
181+
_ => fieldValue.GetString()
182+
};
183+
}
184+
catch (Exception ex) when (ex is InvalidOperationException or FormatException)
185+
{
186+
// this usually means that database column type was changed since generating the GraphQL schema
187+
// for e.g. System.FormatException - One of the identified items was in an invalid format
188+
// System.InvalidOperationException - The requested operation requires an element of type 'Number', but the target element has type 'String'.
189+
throw new DataApiBuilderException(
190+
message: $"The {context.Selection.Field.Name} value could not be parsed for configured GraphQL data type {namedType.Name}",
191+
statusCode: HttpStatusCode.Conflict,
192+
subStatusCode: DataApiBuilderException.SubStatusCodes.GraphQLMapping,
193+
ex);
194+
}
179195
}
180196

181197
return null;

src/Service.Tests/Unittests/ConfigFileWatcherUnitTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
using Microsoft.VisualStudio.TestTools.UnitTesting;
1212
using Moq;
1313

14-
namespace Azure.DataApiBuilder.Service.Tests.Unittests;
14+
namespace Azure.DataApiBuilder.Service.Tests.UnitTests;
1515

1616
[TestClass]
1717
public class ConfigFileWatcherUnitTests

src/Service.Tests/Unittests/MultiSourceQueryExecutionUnitTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
using Microsoft.VisualStudio.TestTools.UnitTesting;
3535
using Moq;
3636

37-
namespace Azure.DataApiBuilder.Service.Tests.Unittests
37+
namespace Azure.DataApiBuilder.Service.Tests.UnitTests
3838
{
3939
[TestClass]
4040
public class MultiSourceQueryExecutionUnitTests

src/Service.Tests/Unittests/MultipleCreateUnitTests/MsSqlMultipleCreateOrderHelperUnitTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
using System.Threading.Tasks;
55
using Microsoft.VisualStudio.TestTools.UnitTesting;
66

7-
namespace Azure.DataApiBuilder.Service.Tests.Unittests
7+
namespace Azure.DataApiBuilder.Service.Tests.UnitTests
88
{
99
[TestClass, TestCategory(TestCategory.MSSQL)]
1010
public class MsSqlMultipleCreateOrderHelperUnitTests : MultipleCreateOrderHelperUnitTests

src/Service.Tests/Unittests/MultipleCreateUnitTests/MultipleCreateOrderHelperUnitTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
using Microsoft.VisualStudio.TestTools.UnitTesting;
1313
using Moq;
1414

15-
namespace Azure.DataApiBuilder.Service.Tests.Unittests
15+
namespace Azure.DataApiBuilder.Service.Tests.UnitTests
1616
{
1717
[TestClass]
1818
public abstract class MultipleCreateOrderHelperUnitTests : SqlTestBase

src/Service.Tests/Unittests/MultipleCreateUnitTests/MySqlMultipleCreateOrderHelperUnitTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
using System.Threading.Tasks;
55
using Microsoft.VisualStudio.TestTools.UnitTesting;
66

7-
namespace Azure.DataApiBuilder.Service.Tests.Unittests
7+
namespace Azure.DataApiBuilder.Service.Tests.UnitTests
88
{
99
/// <summary>
1010
/// Currently, we don't support multiple-create for MySql but the order determination logic for insertions is valid for MySql as well.

src/Service.Tests/Unittests/MultipleCreateUnitTests/PgSqlMultipleCreateOrderHelperUnitTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
using System.Threading.Tasks;
55
using Microsoft.VisualStudio.TestTools.UnitTesting;
66

7-
namespace Azure.DataApiBuilder.Service.Tests.Unittests
7+
namespace Azure.DataApiBuilder.Service.Tests.UnitTests
88
{
99
/// <summary>
1010
/// Currently, we don't support multiple-create for PostgreSql but the order determination logic for insertions is valid for PostgreSql as well.

src/Service.Tests/Unittests/SerializationDeserializationTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
using Azure.DataApiBuilder.Core.Services.MetadataProviders.Converters;
1414
using Microsoft.VisualStudio.TestTools.UnitTesting;
1515

16-
namespace Azure.DataApiBuilder.Service.Tests.Unittests
16+
namespace Azure.DataApiBuilder.Service.Tests.UnitTests
1717
{
1818
[TestClass]
1919
[TestCategory("Serialization and Deserialization using SqlMetadataProvider converters")]

src/Service/Startup.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Azure.DataApiBuilder.Config;
1010
using Azure.DataApiBuilder.Config.Converters;
1111
using Azure.DataApiBuilder.Config.ObjectModel;
12+
using Azure.DataApiBuilder.Config.Utilities;
1213
using Azure.DataApiBuilder.Core.AuthenticationHelpers;
1314
using Azure.DataApiBuilder.Core.AuthenticationHelpers.AuthenticationSimulator;
1415
using Azure.DataApiBuilder.Core.Authorization;
@@ -377,11 +378,16 @@ private void AddGraphQLService(IServiceCollection services, GraphQLRuntimeOption
377378
{
378379
if (error.Exception is DataApiBuilderException thrownException)
379380
{
380-
return error.RemoveException()
381-
.RemoveLocations()
382-
.RemovePath()
381+
error = error.RemoveException()
383382
.WithMessage(thrownException.Message)
384383
.WithCode($"{thrownException.SubStatusCode}");
384+
385+
// If user error i.e. validation error or conflict error with datasource, then retain location/path
386+
if (!thrownException.StatusCode.IsClientError())
387+
{
388+
error = error.RemoveLocations()
389+
.RemovePath();
390+
}
385391
}
386392

387393
return error;

0 commit comments

Comments
 (0)