400 Bad Request if I attach a tag to a system which contains a capital letter
enclave-marc-barry opened this issue · 8 comments
I can tag a system as production but not Production. SDK doesn't handle the error or lowercase my tag, throws an unfriendly exception. With not much thinking, I know tags should be lowercase but this is the kind of thing that might waste a lot of time for another developer. We'll need error handling to be a bit more developer friendly if we want promote external use of this SDK
The API returns the appropriate validation detail:
{
"type": "https://api.enclave.io/problems/type/validation-failure",
"title": "One or more validation failures occurred.",
"status": 400,
"detail": "Please refer to the errors property for additional details.",
"instance": "https://api.enclave.io/problems/instance/de6a24b4f37188ad61adf41f8fa9fa4e",
"errors": {
"tags[0]": [
"The tag 'Production' is not in a valid format. Tags can contain lower-case alphanumeric characters only, optionally separated with hyphens."
]
}
}The API SDK probably needs to handle the problem-details RFC response format we return and surface an exception that contains the errors.
@enclave-tom, this is probably relevant for the golang/terraform stuff you're working on at the moment, because we'll need to surface useful errors from terraform if users muck up their tags.
I thought we'd set the SDK API up to handle RFC response i wonder what's gone on
@enclave-marc-barry, are you able to dump the full contents of the exception you get into this issue?
Details of the Exception stuff can be found here https://github.com/enclave-networks/enclave.sdk.api/tree/main/src/Enclave.Sdk/Exceptions
Sure, if I try to enrol a system with the tag "Management" using the FakeSystemGenerator data app, I'm seeing
Message:
One or more validation failures occurred.
Response:
{StatusCode: 400, ReasonPhrase: 'Bad Request', Version: 1.1, Content: System.Net.Http.HttpConnectionResponseContent, Headers:
{
Server: nginx
Date: Wed, 06 Apr 2022 15:42:04 GMT
Transfer-Encoding: chunked
Connection: keep-alive
Content-Type: application/problem+json; charset=utf-8
}}
StackTrace:
at Enclave.Sdk.Api.Handlers.ProblemDetailsHttpMessageHandler.<SendAsync>d__1.MoveNext()
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
at System.Net.Http.HttpClient.<<SendAsync>g__Core|83_0>d.MoveNext()
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
at Enclave.Sdk.Api.Data.PatchClient`2.<ApplyAsync>d__4.MoveNext()
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
at Enclave.FakeSystemGenerator.Generator.<StartAsync>d__4.MoveNext() in C:\git\enclave\portal\utils\fake-system-generator\Generator.cs:line 98
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Enclave.FakeSystemGenerator.Program.<Main>d__0.MoveNext() in C:\git\enclave\portal\utils\fake-system-generator\Program.cs:line 8
Ok, to be fair, it is in there, just a little buried
Perhaps we could amend the message on the exception to end with "Check the ProblemDetails property on this exception for more information". There's precedent for doing that on some other runtime exceptions that have additional context attached.
Alternatively, we try to be clever, and if there's only one validation error, we just use it as the exception message. That may be the most user-friendly.