From 9aa2519e902ca5663e25ef7dcec108cb12d65ad9 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Tue, 6 Jun 2023 11:32:44 +1200 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20validate=20error=20codes=20on?= =?UTF-8?q?=20client=20side=20(#3131)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Don’t validate error codes on client side * Update docs * Format * Format * Format --- docs/webhook_events.md | 257 +++--------------- .../ApiService/OneFuzzTypes/Model.cs | 14 +- src/ApiService/Tests/ErrorTests.cs | 15 + src/ApiService/Tests/EventsTests.cs | 6 +- src/pytypes/extra/generate-docs.py | 20 +- src/pytypes/onefuzztypes/models.py | 7 +- 6 files changed, 92 insertions(+), 227 deletions(-) create mode 100644 src/ApiService/Tests/ErrorTests.cs diff --git a/docs/webhook_events.md b/docs/webhook_events.md index 0d15ace6f2..2b39c1a2b0 100644 --- a/docs/webhook_events.md +++ b/docs/webhook_events.md @@ -1109,7 +1109,8 @@ If webhook is set to have Event Grid message format then the payload will look a "code": 468, "errors": [ "example error message" - ] + ], + "title": "TASK_FAILED" }, "task_id": "00000000-0000-0000-0000-000000000000", "task_type": "libfuzzer_fuzz" @@ -1130,7 +1131,8 @@ If webhook is set to have Event Grid message format then the payload will look a "Error": { "properties": { "code": { - "$ref": "#/definitions/ErrorCode" + "title": "Code", + "type": "integer" }, "errors": { "items": { @@ -1138,56 +1140,20 @@ If webhook is set to have Event Grid message format then the payload will look a }, "title": "Errors", "type": "array" + }, + "title": { + "title": "Title", + "type": "string" } }, "required": [ "code", + "title", "errors" ], "title": "Error", "type": "object" }, - "ErrorCode": { - "description": "An enumeration.", - "enum": [ - 450, - 451, - 452, - 453, - 454, - 455, - 456, - 457, - 458, - 459, - 460, - 461, - 462, - 463, - 464, - 465, - 467, - 468, - 469, - 470, - 471, - 472, - 473, - 474, - 475, - 476, - 477, - 478, - 479, - 480, - 481, - 482, - 483, - 484, - 485 - ], - "title": "ErrorCode" - }, "JobConfig": { "properties": { "build": { @@ -1762,7 +1728,8 @@ If webhook is set to have Event Grid message format then the payload will look a "code": 472, "errors": [ "example error message" - ] + ], + "title": "PROXY_FAILED" }, "proxy_id": "00000000-0000-0000-0000-000000000000", "region": "eastus" @@ -1777,7 +1744,8 @@ If webhook is set to have Event Grid message format then the payload will look a "Error": { "properties": { "code": { - "$ref": "#/definitions/ErrorCode" + "title": "Code", + "type": "integer" }, "errors": { "items": { @@ -1785,55 +1753,19 @@ If webhook is set to have Event Grid message format then the payload will look a }, "title": "Errors", "type": "array" + }, + "title": { + "title": "Title", + "type": "string" } }, "required": [ "code", + "title", "errors" ], "title": "Error", "type": "object" - }, - "ErrorCode": { - "description": "An enumeration.", - "enum": [ - 450, - 451, - 452, - 453, - 454, - 455, - 456, - 457, - 458, - 459, - 460, - 461, - 462, - 463, - 464, - 465, - 467, - 468, - 469, - 470, - 471, - 472, - 473, - 474, - 475, - 476, - 477, - 478, - 479, - 480, - 481, - 482, - 483, - 484, - 485 - ], - "title": "ErrorCode" } }, "properties": { @@ -2708,7 +2640,8 @@ If webhook is set to have Event Grid message format then the payload will look a "code": 456, "errors": [ "example error message" - ] + ], + "title": "UNABLE_TO_RESIZE" }, "pool_name": "example", "scaleset_id": "example-000" @@ -2723,7 +2656,8 @@ If webhook is set to have Event Grid message format then the payload will look a "Error": { "properties": { "code": { - "$ref": "#/definitions/ErrorCode" + "title": "Code", + "type": "integer" }, "errors": { "items": { @@ -2731,55 +2665,19 @@ If webhook is set to have Event Grid message format then the payload will look a }, "title": "Errors", "type": "array" + }, + "title": { + "title": "Title", + "type": "string" } }, "required": [ "code", + "title", "errors" ], "title": "Error", "type": "object" - }, - "ErrorCode": { - "description": "An enumeration.", - "enum": [ - 450, - 451, - 452, - 453, - 454, - 455, - 456, - 457, - 458, - 459, - 460, - 461, - 462, - 463, - 464, - 465, - 467, - 468, - 469, - 470, - 471, - 472, - 473, - 474, - 475, - 476, - 477, - 478, - 479, - 480, - 481, - 482, - 483, - 484, - 485 - ], - "title": "ErrorCode" } }, "properties": { @@ -3411,7 +3309,8 @@ If webhook is set to have Event Grid message format then the payload will look a "code": 468, "errors": [ "example error message" - ] + ], + "title": "TASK_FAILED" }, "job_id": "00000000-0000-0000-0000-000000000000", "task_id": "00000000-0000-0000-0000-000000000000", @@ -3451,7 +3350,8 @@ If webhook is set to have Event Grid message format then the payload will look a "Error": { "properties": { "code": { - "$ref": "#/definitions/ErrorCode" + "title": "Code", + "type": "integer" }, "errors": { "items": { @@ -3459,56 +3359,20 @@ If webhook is set to have Event Grid message format then the payload will look a }, "title": "Errors", "type": "array" + }, + "title": { + "title": "Title", + "type": "string" } }, "required": [ "code", + "title", "errors" ], "title": "Error", "type": "object" }, - "ErrorCode": { - "description": "An enumeration.", - "enum": [ - 450, - 451, - 452, - 453, - 454, - 455, - 456, - 457, - 458, - 459, - 460, - 461, - 462, - 463, - 464, - 465, - 467, - 468, - 469, - 470, - 471, - 472, - 473, - 474, - 475, - 476, - 477, - 478, - 479, - 480, - 481, - 482, - 483, - 484, - 485 - ], - "title": "ErrorCode" - }, "StatsFormat": { "description": "An enumeration.", "enum": [ @@ -5530,7 +5394,8 @@ If webhook is set to have Event Grid message format then the payload will look a "Error": { "properties": { "code": { - "$ref": "#/definitions/ErrorCode" + "title": "Code", + "type": "integer" }, "errors": { "items": { @@ -5538,56 +5403,20 @@ If webhook is set to have Event Grid message format then the payload will look a }, "title": "Errors", "type": "array" + }, + "title": { + "title": "Title", + "type": "string" } }, "required": [ "code", + "title", "errors" ], "title": "Error", "type": "object" }, - "ErrorCode": { - "description": "An enumeration.", - "enum": [ - 450, - 451, - 452, - 453, - 454, - 455, - 456, - 457, - 458, - 459, - 460, - 461, - 462, - 463, - 464, - 465, - 467, - 468, - 469, - 470, - 471, - 472, - 473, - 474, - 475, - 476, - 477, - 478, - 479, - 480, - 481, - 482, - 483, - 484, - 485 - ], - "title": "ErrorCode" - }, "EventCrashReported": { "properties": { "container": { diff --git a/src/ApiService/ApiService/OneFuzzTypes/Model.cs b/src/ApiService/ApiService/OneFuzzTypes/Model.cs index f59b83358c..374e9622c3 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Model.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Model.cs @@ -168,11 +168,17 @@ bool Outdated ) : StatefulEntityBase(State); public record Error(ErrorCode Code, List? Errors) { - public static Error Create(ErrorCode code, params string[] errors) { - return new Error(code, errors.ToList()); - } + // A human-readable version of the ErrorCode, + // so that when serialized to JSON there is something useful, + // not just a number. This is named 'Title' to align with the + // ProblemDetails class. + public string Title => Code.ToString(); + + public static Error Create(ErrorCode code, params string[] errors) + => new(code, errors.ToList()); + public sealed override string ToString() { - var errorsString = Errors != null ? string.Join("", Errors) : string.Empty; + var errorsString = Errors != null ? string.Concat("; ", Errors) : string.Empty; return $"Error {{ Code = {Code}, Errors = {errorsString} }}"; } }; diff --git a/src/ApiService/Tests/ErrorTests.cs b/src/ApiService/Tests/ErrorTests.cs new file mode 100644 index 0000000000..186ca00851 --- /dev/null +++ b/src/ApiService/Tests/ErrorTests.cs @@ -0,0 +1,15 @@ +using System.Text.Json; +using Microsoft.OneFuzz.Service; +using Xunit; + +namespace Tests; + +public class ErrorTests { + + [Fact] + public void JsonHasErrorTitle() { + var error = Error.Create(ErrorCode.INVALID_IMAGE); + var json = JsonSerializer.Serialize(error); + Assert.Equal(@"{""Code"":463,""Errors"":[],""Title"":""INVALID_IMAGE""}", json); + } +} diff --git a/src/ApiService/Tests/EventsTests.cs b/src/ApiService/Tests/EventsTests.cs index 4090e8a9f9..cf04051da9 100644 --- a/src/ApiService/Tests/EventsTests.cs +++ b/src/ApiService/Tests/EventsTests.cs @@ -7,9 +7,9 @@ namespace Tests; public class EventTests { [Fact] - static void CheckAllEventClass() { + public static void CheckAllEventClass() { // instantiate one event to force the static constructor to run - var testEvent = new EventPing(Guid.Empty); - + // if it doesn't throw then this test passes + _ = new EventPing(Guid.Empty); } } diff --git a/src/pytypes/extra/generate-docs.py b/src/pytypes/extra/generate-docs.py index 799cd042d5..50dfc0ea51 100755 --- a/src/pytypes/extra/generate-docs.py +++ b/src/pytypes/extra/generate-docs.py @@ -152,7 +152,11 @@ def main() -> None: EventTaskFailed( job_id=UUID(int=0), task_id=UUID(int=0), - error=Error(code=ErrorCode.TASK_FAILED, errors=["example error message"]), + error=Error( + code=ErrorCode.TASK_FAILED.value, + title="TASK_FAILED", + errors=["example error message"], + ), user_info=UserInfo( application_id=UUID(int=0), object_id=UUID(int=0), @@ -171,7 +175,11 @@ def main() -> None: EventProxyFailed( region=Region("eastus"), proxy_id=UUID(int=0), - error=Error(code=ErrorCode.PROXY_FAILED, errors=["example error message"]), + error=Error( + code=ErrorCode.PROXY_FAILED.value, + title="PROXY_FAILED", + errors=["example error message"], + ), ), EventProxyStateUpdated( region=Region("eastus"), @@ -197,7 +205,9 @@ def main() -> None: scaleset_id="example-000", pool_name=PoolName("example"), error=Error( - code=ErrorCode.UNABLE_TO_RESIZE, errors=["example error message"] + code=ErrorCode.UNABLE_TO_RESIZE.value, + title="UNABLE_TO_RESIZE", + errors=["example error message"], ), ), EventScalesetDeleted(scaleset_id="example-000", pool_name=PoolName("example")), @@ -231,7 +241,9 @@ def main() -> None: task_id=UUID(int=0), task_type=TaskType.libfuzzer_fuzz, error=Error( - code=ErrorCode.TASK_FAILED, errors=["example error message"] + code=ErrorCode.TASK_FAILED.value, + title="TASK_FAILED", + errors=["example error message"], ), ), JobTaskStopped( diff --git a/src/pytypes/onefuzztypes/models.py b/src/pytypes/onefuzztypes/models.py index 64969c6a43..e6a7696c52 100644 --- a/src/pytypes/onefuzztypes/models.py +++ b/src/pytypes/onefuzztypes/models.py @@ -18,7 +18,6 @@ Compare, ContainerPermission, ContainerType, - ErrorCode, GithubIssueSearchMatch, GithubIssueState, HeartbeatType, @@ -95,7 +94,11 @@ def exactly_one(cls: Any, values: Any) -> Any: class Error(BaseModel): - code: ErrorCode + # the code here is from ErrorCodes.cs, but we don't + # want to validate the error code on the client-side + code: int + # a human-readable version of the error code + title: str errors: List[str]