Remora / Remora.Discord

A data-oriented C# Discord library, focused on high-performance concurrency and robust design.
GNU Lesser General Public License v3.0
246 stars 44 forks source link

[Bug]: ExceptionErorr while deserializing json for webhook message after passing wait=true parameter #313

Closed b-rad15 closed 5 months ago

b-rad15 commented 10 months ago

Description

While deserializing the author portion of a webhook message response, there is an error that GlobaName was expected but not found: "ExceptionError { Message = The data property "GlobalName" did not have a corresponding value in the JSON., Exception = System.Text.Json.JsonException: The data property "GlobalName" did not have a corresponding value in the JSON."

Steps to Reproduce

Here is the exact test code I was using in xunit to debug this:

await SetupDiscordService();
IConfigurationRoot configuration = new ConfigurationBuilder()
    .AddJsonFile("appsettings.json")
    .Build();
//Extract Webhook Params
var webhookUrl = configuration["TestWebhookChannel"];
Assert.NotNull(webhookUrl);
var webhookRegexMatch = RegexData.WebhookRegex().Match(webhookUrl);
var webhookToken = webhookRegexMatch.Groups["Token"].Value;
var webhookIdString = webhookRegexMatch.Groups["Id"].Value;
Snowflake webhookId = new(ulong.Parse(webhookIdString));
Assert.NotNull(DiscordHostService);
//Send The Message
var webhookService = DiscordHostService.Services.GetRequiredService<IDiscordRestWebhookAPI>();
var webhookSendResult =
    await webhookService.ExecuteWebhookAsync(webhookId, webhookToken, true, $"Test Message at {DateTime.Now}", $"Test Username at {DateTime.Now}");
//Check the result
if (!webhookSendResult.IsSuccess)
{
    _testOutputHelper.WriteLine($"Failed to send webhook message: {webhookSendResult.Error}");
    Assert.True(webhookSendResult.IsSuccess);
}

Expected Behavior

GlobalName should be an Optional<string?> value of an IUser object despite Discord incorrectly marking this as a required nullable string in their API docs: https://discord.com/developers/docs/resources/user#user-object

Current Behavior

Discord responds with the following after waiting for a webhook message (Ids redacted just in case):

"author": {
    "id": "redacted",
    "username": "test username postman",
    "avatar": null,
    "discriminator": "0000",
    "public_flags": 0,
    "flags": 0,
    "bot": true
    },

IUser has a GlobalName property that is string? meaning that when global_name is not present on the author response the json deserializer throws an exception.

Library / Runtime Information

framework: net7.0 Remora.Discord: 2023.4.0 Remora.Discord.Rest: 48.0.0

b-rad15 commented 10 months ago

Seems this is a reported bug on the dev docs as well under this link https://github.com/discord/discord-api-docs/issues/6347

Nihlus commented 10 months ago

Since this is a documentation problem on Discord's end, I'm remiss to change Remora's implementation until that is resolved. What does their OpenAPI spec say about the entity/endpoint?

b-rad15 commented 10 months ago

it has this for a response on the execute webhook action https://github.com/discord/discord-api-spec/blob/main/specs/openapi.json#L8361-L8371 :

"responses": {
  "200": {
    "description": "200 response for execute_webhook",
    "content": {
      "application/json": {
        "schema": {
          "$ref": "#/components/schemas/MessageResponse"
        }
      }
    }
  },
  "204": {
    "description": "204 response for execute_webhook"
  },
  "4XX": {
    "$ref": "#/components/responses/ClientErrorResponse"
  }
},

and this as the relevant portion of what a MessageReference is https://github.com/discord/discord-api-spec/blob/main/specs/openapi.json##L22472-L22474 :

"MessageResponse": {
  "type": "object",
  "properties": {
    "id": {
      "type": "string",
      "format": "snowflake"
    },
    "type": {
      "$ref": "#/components/schemas/MessageType"
    },
    "content": {
      "type": "string"
    },
    "channel_id": {
      "type": "string",
      "format": "snowflake"
    },
    "author": {
      "$ref": "#/components/schemas/UserResponse"
    },
    ...

and lastly UserResponse https://github.com/discord/discord-api-spec/blob/main/specs/openapi.json##L26673-L26738:

"UserResponse": {
  "type": "object",
  "properties": {
    "id": {
      "type": "string",
      "format": "snowflake"
    },
    "username": {
      "type": "string"
    },
    "avatar": {
      "type": [
        "string",
        "null"
      ]
    },
    "discriminator": {
      "type": "string"
    },
    "public_flags": {
      "type": "integer",
      "minimum": -2147483648,
      "maximum": 2147483647,
      "format": "int32"
    },
    "flags": {
      "type": "integer",
      "minimum": -9007199254740991,
      "maximum": 9007199254740991
    },
    "bot": {
      "type": [
        "boolean",
        "null"
      ]
    },
    "system": {
      "type": [
        "boolean",
        "null"
      ]
    },
    "banner": {
      "type": [
        "string",
        "null"
      ]
    },
    "accent_color": {
      "type": [
        "integer",
        "null"
      ],
      "minimum": -2147483648,
      "maximum": 2147483647,
      "format": "int32"
    }
  },
  "required": [
    "id",
    "username",
    "discriminator",
    "public_flags",
    "flags"
  ]
}

So their API spec just doesn't have global_name in it at all as far as I can see. Having global_name means the documentation is just out of spec while also just being wrong for this (and others from a quick issues search on their Github) endpoint. I understand wanting to be in spec with their docs and not be chasing their errors but it does make certain functions just non-functional.

Unless there is a way to allow for missing json keys (leaving the keys null I guess?) I don't think there's a less disruptive fix.

Nihlus commented 10 months ago

I would actually go with what the OpenAPI spec says in this case. Remove the property outright and leave a note somewhere in a comment that it's intentional.

b-rad15 commented 5 months ago

This has been resolved by #316 and later #325