discord/discord-api-spec

`/users/@me/guilds`'s `before` has irrelevant `null` type

A5rocks opened this issue · 8 comments

At the moment, the specification says:

{
"name": "before",
"in": "query",
"schema": {
"type": [
"string",
"null"
],
"format": "snowflake"
}
},

However, before cannot be sent as null (see comments for why).

(edited because I'm blind and missed the "format": "snowflake" cause that's not what I was looking for :( )

Why do you say it cant be null?

The docs here indicate it can, and the code certainly allows it.

Sorry, I think I might be missing something: I don't see how the docs indicate it can. Additionally, I get this error when using ?before=null:

{
	"message": "Invalid Form Body",
	"code": 50035,
	"errors": {
		"before": {
			"_errors": [
				{
					"code": "NUMBER_TYPE_COERCE",
					"message": "Value \"null\" is not snowflake."
				}
			]
		}
	}
}

(I was about to include this in the issue but didn't for terseness reasons... maybe I should have xd)

Thats because before=null in a query argument sends the string 'null', not an actual null value. In query args, null is equivalent to optional

Yeah I wasn't sure about that, my bad. I checked against https://swagger.io/docs/specification/describing-parameters/#query-parameters for a quick sanity check (i.e. that query parameters can be non-string -- in the example, the parameters are typed as integer and accept integers) but I guess this is just another thing to be aware of when interpreting this specification?

First, just a quick note: Those swagger docs are for OpenAPI 3.0, the actual reference for 3.1 is https://spec.openapis.org/oas/v3.1.0

To answer your question though: You can (and we do) have non-string types in query arguments, but if you consider where they're coming from - the URL - you have to understand that the only thing you can represent in the URL is a value serialized as a string. Lets look at a few examples:

type: string
query_value: hi
parsed_output: 'hi'

type: number
query_value: 1
parsed_output: 1

type: string
query_value: 1
parsed_output: '1'

type: string
query_value: null
parsed_output: 'null'

type: number
query_value: foo
parsed_output: ERROR 'foo' cannot be parsed as a number

Hopeully this explains why before=null is not correct input despite the fact that before is nullable. And this is not a special case of our spec, this is just.. idk facts of life.

I still don't quite get this TBF (I get why null as string leads this way, but not null as string | null) but I guess why something is interpreted as something else is out of scope (though I do like to learn about this stuff from conversations like this!).

I'll go ahead and change the issue to focus exclusively on how null is irrelevant (you can't give the system a null).

but not null as string | null

Simply because there is no actual way to send a null in the query string. In a request body, you can send a null by telling the API that your request is encoded in a certain way (usually application/json), and then JSON itself specifies how to send null (in JSON, null is null and the string is "null" right).

before has irrelevant null type

So this part is a quirk of our spec that we don't differentiate null and optional, for which there are underlying technical limitations forcing our hand here. However, we could probably always remove null as an option from query args specifically, so that's maybe a TODO for us.

should be fixed