#Development Chat

1 messages · Page 4 of 1

silent marsh
#

i'll push it, feel free to revert if you don't like it

eternal owl
#

RestrictedBool?

wild marlin
#

I like that name more, I named it MustBe because that was what it was called in the other create.

karmic aurora
silent marsh
#

i think that would be @wild marlin

wild marlin
#

Should be fun

#

But I will give it a go

#

I have some time later

silent marsh
#

awesome

#

btw for guild scheduled events stuff I have a commit on my fork somewhere for exposing it in the cache

#

it needs docs changes because I think the intent is a different one than I currently documented

wild marlin
#

@karmic aurora Should We just try and merge it?

#

Looks like it needs to be rebased

#

Also if you are looking at it can you add workflow_dispatch: to the workflow as well?

#

I really don't like having it around as open if we cannot agree to merge it in any form.

wild marlin
#

Only new thing is the last commit

#

(and a rebase, but that went without issues)

karmic aurora
silent marsh
silent marsh
wild marlin
wild marlin
#

I will delete the gh-pages branch

#

It seems to work

silent marsh
#

nvm it does if I manually use https://

wild marlin
#

I can set it to enforce TLS if we want that, though tbh I don't think it is too important

silent marsh
#

hmmmmm

wild marlin
#

Hmm actually

Enforce HTTPS — Unavailable for your site because your domain is not properly configured to support HTTPS

#

I will have to look into that

swift gull
#

Interesting, it's using google trust services. Considering the IP is CF, is it using a fallback cert?

silent marsh
#

google trust services is what cf uses for certs

wild marlin
#

I have tried to turn of ssl on cloudflare for the api subdomain

#

We will see if that fixes it?

silent marsh
#

what's your DNS setup

#

for my github pages stuff I add a cname file and a cname record to github.io afair

wild marlin
#

This happens because we do some stuff with cloudflare

#

It takes quite a while to propagate I think, I will have another look tomorrow afternoon if it is still being annoying.

#

Oh I think I fixed it

#

Now github just have to update ther cert or something

#

Okay I turned off cloudflare proxy for the entry so it should be fixed in a bit

#

Now it is fixed @silent marsh, @swift gull Cloudflare proxied it which caused that.

silent marsh
#

yup all fine now

#

am I missing something or is this unused

wild marlin
#

Oh yeah, looks like it is missing a

["applications", id, "emojis", _emoji_id] => ApplicationEmoji(parse_id(id)?),
autumn sequoia
#

Would've been great if y'all could've arrived at that conclusion a year ago. Say, in April.

#

Oh well.

wild marlin
#

Thanks, I am very sorry about this all went, this should not happen and is just detrimental to a project and contributors.

silent marsh
#

I think the issue was that we were in a fundamental disagreement regarding the purpose of the PR and don't really have a way of dealing with that. There's no rules that say e.g. a majority of the developers needs to agree or that it must be unanimous. Ideally we wouldn't need such rules, because it has been working well for us until now. This PR was the only exception that I'm aware of. I perhaps should've voiced my disagreement with the PR on Github rather than Discord.

#

I'm very sorry that the pull request was left open without any updates to it from our side, especially from my end.

eager birch
#

Yeah, i was really hoping for that to land in 0.16. I hate having potentially wrong cryptographic code sitting in my own crates.

lean geyser
#

i dont know how easy to misuse or do wrong this would actually be. If you decode the bytes wrong, validation fails. use the wrong public key or key encoding? verification fails.

eager birch
#

hmm, that's a good point. this isn't like encryption where you can fail-unsafe

primal torrent
#

And discord actually verifies you did it right

eager birch
#

yeah

#

it does ring alarm bells tho

#

"why am i using this crate i don't understand to provide security"

primal torrent
eager birch
#

i'm not interacting with the fragile parts of those. I trust the developers of the abstractions, and I'm just not confident in how abstract the ed25519-dalek crate is

primal torrent
#

Generally you only need to provide the key and the signed content. Doesn't seem fragile to me

#

Its not like these algorithms ever change

eager birch
#

sure. i'm just used to the golden rule of cryptography being "don't touch it if you don't know what you're doing"

primal torrent
#

Agreed, but you're only validating

eager birch
#

i get why it isn't in twilight- it doesn't really save any code anyway- it's just a little weird to be writing code that directly does cryptographic Anything

primal torrent
#

Maybe a standalone lib would be useful

eager birch
#

i feel like that's the kind of crate that would be considered so small that it's not worth using it

#

and that would go unmaintained after a month

primal torrent
#

But its also something most libs dont provide

#

Only full framework

eager birch
#

(to be honest, just using websocket is better in a lot of twilight-related use cases. but http is still something twilight supports first-class, which is very nice)

primal torrent
#

Some framework that handles web would be nice but imo should he the full thing and build on top rather then in twilight

eager birch
#

i want me an axum-like slash command framework :3

#

sadly i know so little about effective rust library development i can't really do that

#

plus the way the Interaction is structured it's hard to make generally useful extractors.

autumn sequoia
#

I did offer to write middlewares for common web server frameworks that do Discord signature validation, so using it would've boiled down to something like .layer(DiscordSignatureValidation::with_hex_key("DISCORD_PUB_KEY_HERE")), but that was rejected. Basically this proposal was divided and conquered: cut down to the minimum generic thing, and then shot down for not doing enough.

Which, whatever, I just wish I hadn't wasted the time, and I'm glad I didn't tolerate the drawn out review process longer than I did, since it was apparently doomed from the start.

I did, however, mention my intention to make a crate for doing this in the GH, if the PR didn't end up merged. I don't have the time to do that right now, my plate is currently full, but I promised I would do it, so it is coming eventually. Probably this year.

#

Oh, and, for the record, the validation that Discord does of your signature checks is easy to defeat without doing correct usage of ed25519-dalek, so your fears are real.

#

lol

wild marlin
autumn sequoia
# wild marlin If you want some help figuring stuff out for the middle ware feel free to take c...

I have a general understanding of how to do this sort of stuff, see my previous work for a demo, it'll just be a matter of making everything polished and properly verifying my implementations.

I would still appreciate being able to talk about this stuff and maybe get some help with maintenance, or recruiting maintainers, down the line, though.

Source Cat: Git with a ball of yarn
twilight-signature-validation

A temporary crate for testing out utilities I hope get merged into twilight-util.

eager birch
#

i suppose i need to audit my usage.

autumn sequoia
# eager birch oh dear. how so?

Well, all Discord does to validate you're checking signatures is send two requests right next to each other, one valid and one not. It does this once when you register the endpoint to receive Interactions, and then occasionally after that. It makes sure that you're doing something, but not that you're doing the right thing.

#

realistically, if you're doing something like this: ```rs
let Some(signed_buf) = headers.get("x-signature-timestamp") else {
return Err((StatusCode::BAD_REQUEST,
"request did not include signature timestamp header".to_string()))
};
let mut signed_buf = signed_buf.as_bytes().to_owned();
signed_buf.extend_from_slice(body.as_bytes());

let pub_key = discord_pub_key();

dbg!(pub_key.verify_strict(&signed_buf, &sig));

you're doing the right thing.
eager birch
#

yeah, pretty much what i'm doing

#

it is annoying because you can't easily do it in middleware because Body Is A Stream but Oh Well

autumn sequoia
#

it's not impossible to do in a middleware though. can you say what web server library you're using, by the way? if it's not axum, I'll add it to the list of ones I'll make sure to support in the crate I promised

#

(axum is already on the list)

silent marsh
eager birch
#

yes, you can, it's just annoying

autumn sequoia
#

yeah

eager birch
#

I love axum oh my gosh it's fantastic

autumn sequoia
#

nice

karmic aurora
#

I'm finally resuming my work on adding bucket support to the http ratelimiter, and the code's so nice and easy that I don't understand why I procrastinated so much… Attached is a teaser 👀

📎 ratelimiter.rs
#

Oh and I'm also adding global ratelimit checks (max 50 msg/s)

silent marsh
#

DelayQueue 🙏

#

didn't exist back when this was originally written

#

same for JoinSet actually, tokio-util has evolved a lot

wild marlin
silent marsh
#

Should be ready to merge now

#

nvm test fail fixed

silent marsh
#

could I perhaps get a review on the rustls 0.23 PR so that one can get merged?

#

also @wild marlin could you rebase the user apps stuff so that CI will run all jobs?

#

Or let me know if you don't have time, I can do it as well but don't wanna push to your branch without permission

wild marlin
#

I have time to do it tomorrow

#

or well later today

silent marsh
#

great, I should have time to review it later today as well

#

can't say much except looking at API docs but well, I'll try

wild marlin
silent marsh
#

No worries

#

Thanks, I'll take a look in the afternoon

wild marlin
#

I don't have much time until next weekend as I am finishing up a presentation for a rust meetup, but after that I can look into some of the blocking issues.

silent marsh
#

did someone ever reply to my proposal a while ago about moving lavalink out of the monorepo

#

the more I think about it, the more I think it should be developed separately (that's not to say the v4 lavalink update shouldn't be done)

#

i don't think any of the developers here use it, so it's mostly untested and slapping an official twilight label on it just doesn't seem right, at least at the moment

primal torrent
#

i think it could be under the twilight org and still be official. but more of an separate addon then the core twilight repo

#

that said it might be best to do the v4 and split off after to reduce overhead?

silent marsh
#

I would prefer it somewhere else and maintained by someone who actually uses it. Moving it to a different repo under the twilight org would only incentivize abandoning it even more

#

And yes, I would suggest splitting it off after v4

silent marsh
#

huh, I must've missed that Discord allows zstd gateway compression now

valid oracle
#

They made a recent blog post about it, but I don't think they announced it via dev stuff

wild marlin
#

@karmic aurora Do you just want it inside of the mod.rs then?

#

And adding a module is only backwards compat if we reexport it, though that should not be an issue

karmic aurora
wild marlin
#

@karmic aurora Both are ready for one more look

#

I somehow double closed the issue in #github-log

wild marlin
opal juniper
#

Thank you!

wild marlin
#

Anymore who wants to look at user apps before I merge it later today?

#

(looking at you @ocean nest)

ocean nest
#

i mean ig u dont agree with my suggestions but its fine

wild marlin
#

Oh I should probably write a reason

#

It's mostly that it would allow some inputs which was not correct if you used that way of doing it.

#

Like matching up a UserInstall with a Guild which should be allowed

wild marlin
#

Crosses fingers that not everything blows up now

ocean nest
#

thats just programming

ocean nest
#

yes the traits are alot of boilerplate but still

wild marlin
#

Not in a way that would ensure it at compile time

wild marlin
#

I have a couple of open prs if anyone is bored

wild marlin
#

Another small pr for the list

primal torrent
#

i'll try to get through some of them today

wild marlin
#

Anyone else have a bit of time for the two most recent prs?

gaunt pollen
#

Will check in a few hours

gaunt pollen
#

The rustls default CryptoProvider not set error is a bit annoying pain

silent marsh
#

yeah i should PR something against hyper-rustls to fix that

karmic aurora
#

I'll fix the CI errors that popped up in my latest pr tomorrow morning, unless anyone does so before then

wild marlin
#

@karmic aurora Is it not a bit early to solve 1.84 errors?n

karmic aurora
#

My bad, that's a typo

wild marlin
#

Hehe, though there have been calls for 1.84 testing with the new edition and everything

#

Also I still think that our minimum supported version should be bumped to the lastest when we actually cut a release.

wild marlin
#

So who wants to have a look at rate limiting?

gaunt pollen
eager birch
fluid dust
#

not true lol

#

i've gotten rate-limited a lot because of this

lone current
#

I swear I remember it being like 100 changes a day

#

Or something like that

eager birch
primal torrent
#

Problem is its inconsistent and hard to debug. So there have been very little to no repro cases for staff to debug and fix

eager birch
#

suffering

primal torrent
#

Oh also keep the nex start activity command in mind

#

Seen that blow up lots of sync implementations

eager birch
#

what on earth is that

primal torrent
# eager birch what on earth is that

If you enable activities, it adds an activity command to start it. It can be updated but not removed by the bulk endpoint, only the individial command endpoint

eager birch
#

i think i’m going to ignore that

primal torrent
#

The issue is the call fails and nothing gets updated

#

Wasn't a fun time getting all those who turned on activities to mess around with to turn it off again

eager birch
primal torrent
#

a classical case of discord knowing full well how most of the ecosystem works, yet not caring about breaking it without warning

eager birch
#

sigh

#

yeah…

eager birch
#

no tests yet, just wrote out what i think is all the annoying boilerplate

stoic umbra
#

I'm not sure which cases this could be helpful since there already exists bulk overwrite application commands api

#

also, to check if commands are different, I think we just need to check the command ID and its version nvm

eager birch
stoic umbra
eager birch
#

how long have you been working on discord bots

#

discord is somewhat infamous for seeing issues like this and just saying "skill issue"

#

like, were that true, and it were likely to be fixed shortly, i wouldn't have spent the tedious five hours writing all this silly sort-and-compare code

stoic umbra
primal torrent
karmic aurora
silent marsh
#

would love if someone could verify that though

#

mmh yeah that does fix it

wild marlin
#

Nice they were pretty responsive

gaunt pollen
#

👏

silent marsh
#

I'm really happy, it's a massive improvement over what the situation used to be like on that repo

wild marlin
#

@karmic aurora I am not sure I can prove it can be an issue, but I am not a fan of using the capacity of a vec in a way that is important for correctness

#

I think that part was not changed by your optimisation which is why I just write it here.

#

Also especially this note on Vec::with_capacity:

The vector will be able to hold at least capacity elements without reallocating. This method is allowed to allocate for more elements than capacity [the variable].

karmic aurora
#

It's currently equivalent, but it may not be in the future. Nice catch

wild marlin
#

It feels like it should use a ring buffer, but that is a optimization that can be done at some other point because I think it is a bit of work if you don't use a external crate.

karmic aurora
#

Yeah, VecDeque::partition_point did not previously work, but that's no longer an issue. Drain should now just update the head pointer without moving any items

karmic aurora
# wild marlin Also especially this note on `Vec::with_capacity`: > The vector will be able to ...

Actually, reserve_exact has the same issue:

Note that the allocator may give the collection more space than it requests. Therefore, capacity can not be relied upon to be precisely minimal. Prefer reserve if future insertions are expected.

It's seemingly impossible to allocate a Vec/VecDeque with exact capacity for all custom allocators https://users.rust-lang.org/t/reserve-truly-exact-capacity-for-vec/109751

See Allocator::allocate returning a slice vs GlobalAlloc::alloc returning a raw pointer. My understanding is that GlobalAlloc::alloc will allocate the requested size (as given by reserve_exact/with_capacity and corresponding with malloc), but that's not true for custom allocators which may return slices larger than requested:

On success, returns a NonNull<[u8]> meeting the size and alignment guarantees of layout.

The returned block may have a larger size than specified by layout.size(), and may or may not have its contents initialized.

We use the global allocator, ergo we can trust with_capacity

#

See also the second example here trusting the capacity from the global allocator

wild marlin
#

@karmic aurora Just add a usize to the struct containing the calculated length, that is probably better.

#

I thought reserve exact would work tbh.

karmic aurora
#

I added an assert that the capacity is exact. Worst case scenario the library teams notify us from a failing crater run if they were to change the behavior

wild marlin
#

Can you add a note to the push that notes that it important to uphold that the queue can never grow?

#

tbh I have run my bot with other allocators quite a bit so the GlobalAllocator assumption might be a bit too much

karmic aurora
#

I think the only way to install global allocators is by implementing the GlobalAlloccator trait, which does not allow allocators to oversize allocations (as only a raw pointer is returned)

valid oracle
#

Just PR to rust-lang/rust to add the guarantee to documentation, if you want to rely on it.

#

don't purposely get yourself hit in a crater run

gaunt pollen
#

I didn't have time to review it yet, but it for sure sounds like a bad idea

#

Just store the queue size in a separate variable.
Those few bytes more are better than an unstable assert

wild marlin
#

I wonder what the crate that get hit most often in crater runs is.

karmic aurora
#

I added a proper, documented, solution following this example (found on Vec::into_boxed_slice):

let mut vec = Vec::with_capacity(10);
vec.extend([1, 2, 3]);

assert!(vec.capacity() >= 10);
let slice = vec.into_boxed_slice();
assert_eq!(slice.into_vec().capacity(), 3);
gaunt pollen
#

But here it is checking for >=

karmic aurora
#

Initially, but the capacity is exactly 3 after into_boxed_slice().into_vec()

gaunt pollen
wild marlin
#

I think I agree with gnome apperently using capacity for correctness is just foot gun upon footgun

karmic aurora
gaunt pollen
jaunty jewel
#

2282 merged, 2235 superceded by 2237 and merged
2187... idk
2253 closed
2159 is still open
2162 seems to still be wip

wild marlin
#

Anyone wants to look into ratelimits?

silent marsh
#

In all honesty, the ratelimiter needs a full on rewrite

wild marlin
#

Yeah I agree

silent marsh
#

as far as I recall we're not using the new response headers Discord sends for ratelimit buckets, but I'm not up to date on that either

#

And no global ratelimit handling, etc.

gaunt pollen
#

/user/ is a globally rate limited endpoint

#

Iirc

primal torrent
#

The globak rate limit is always at least 50/s so that is rarely the issue fir bots

valid oracle
#

serenity needs a rate limit implementation as well, it would be nice to be a separate library so it could be shared tbh

wild marlin
#

Though it probably needs to be changed around a bit.

jaunty jewel
jaunty jewel
silent marsh
#

For escaping you'll probably want something like https://docs.rs/v_escape/latest/v_escape/

v_escape - Rust

Crate v_escape provides a macro, new! that define a struct with escaping functionality. These macros are optimized using simd by default, but this can be alter using sub-attributes.

jaunty jewel
silent marsh
#

I think it's okay to limit to &str

silent marsh
jaunty jewel
#

well I probably should upgrade fmt to a directory module then

#

cuz I need to add escape lol

karmic aurora
# 
let frame = (code == CloseCode::NO_STATUS_RECEIVED).then(|| CloseFrame {
    code: code.into(),
    reason: Cow::Owned(reason.to_string()),
});
karmic aurora
wild marlin
#

It is not even valid code

karmic aurora
#

The account is 4 days old, it's some form of spam/advertisement

wild marlin
karmic aurora
#

I believe the inflater needs to be in sync with discord, i.e. we'd need to reconnect to restart the shared context

#

Otherwise I'd expect us to inflate junk (as the compression algorithm depends upon previous sent data)

wild marlin
#

Yeah if the inflater gets an error we already do a resume because of this

karmic aurora
wild marlin
#

Hmm I was sure I had made that at some point, it might have disaapeared at some point.

#

Okay so it has disappeared somewhere since 0.14.2, it used to be a reconnectable error so it would not even do a resume.

karmic aurora
#

I'll re-add it into my zstd branch

silent marsh
opal juniper
#

Do y’all need help with anything I’m free this weekend

karmic aurora
#

Someone needs to update the 0.16 changelog eventually

wild marlin
#

Or u have done it already

#

@silent marsh @karmic aurora Should we write up a list of the things that are necessary for a 0.16 release?

#

And Adrian we should have a section in the book about tls I think

karmic aurora
jaunty jewel
#

yeah i think we can do that

wild marlin
#

I don't think we would get much more feedback from a second release candidate.

#

@karmic aurora Hmm I might be wrong about the new reset comment, though I think I would check the zstd docs.

#

Hmm I think the you need it to reset the internal dict

#

Its a bit confusing since they seemingly don't reference clearly.

#

But I am pretty sure it does need the parameters to be reset if it is possible without creating a new DCtx

#

The ZSTD_clearDict also clears ddictLocal which is the dictionary that is used if no other is set.

#

Also where is the quote you posted there from?

#

oh the blog post.

karmic aurora
wild marlin
wild marlin
#

I marked the issue as resolved.

silent marsh
silent marsh
wild marlin
#

Will you write up a book chapter?

silent marsh
#

I can do that, yes, unless someone else really feels like doing it

#

We already have one for the release candidate, it needs to be updated and renamed

wild marlin
#

I can do a release this weekend

wild marlin
silent marsh
wild marlin
# 
        for (name, value) in parts.headers {
            if let Some(name) = name {
                if tokio_websockets::ClientBuilder::DISALLOWED_HEADERS.contains(&name) {
                    continue;
                }
                builder = builder.add_header(name, value)?;
            }
        }

This causes:

error[E0283]: type annotations needed
   --> ocpp/src/websocket.rs:66:20
    |
66  |                 if tokio_websockets::ClientBuilder::DISALLOWED_HEADERS.contains(&name) {
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type of the type parameter `R` declared on the struct `Builder`
    |
    = note: cannot satisfy `_: tokio_websockets::resolver::Resolver`
    = help: the trait `tokio_websockets::resolver::Resolver` is implemented for `Gai`
    = note: associated constants cannot be accessed directly on a `trait`, they can only be accessed through a specific `impl`
note: required by a bound in `ClientBuilder::<'a, R>::DISALLOWED_HEADERS`
   --> /home/erk/.cargo/git/checkouts/tokio-websockets-ca6803b6174f5975/9e090bf/src/client.rs:155:13
    |
155 | impl<'a, R: Resolver> Builder<'a, R> {
    |             ^^^^^^^^ required by this bound in `ClientBuilder::<'a, R>::DISALLOWED_HEADERS`
...
164 |     pub const DISALLOWED_HEADERS: &'static [HeaderName] = &[
    |               ------------------ required by a bound in this associated constant
help: consider specifying the generic argument
    |
66  |                 if tokio_websockets::ClientBuilder::<R>.contains(&name) {
    |                                                   ~~~~~
help: use the fully qualified path to an implementation
    |
66  |                 if <Type as tokio_websockets::resolver::Resolver>::DISALLOWED_HEADERS.contains(&name) {
    |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

For more information about this error, try `rustc --explain E0283`.
silent marsh
#

Eh, seems fine?

#

Does it compile if you explicitly specify the generic?

wild marlin
#

tokio_websockets::ClientBuilder::<tokio_websockets::resolver::Gai>::DISALLOWED_HEADERS.contains(&name) { Yeah I guess this works

silent marsh
#

Yeah, it can't guess the resolver if you're using an associated constant it seems

#

That seems like a compiler limitation

wild marlin
#

Yeah I am not sure there is any nice way of resolving it for now.

silent marsh
#

The easy way would be moving the constant out of the struct

#

I wouldn't say that I like it, but it's probably nicer than having to specify the generic

wild marlin
#

Yeah probably, and just put it top level in the module.

silent marsh
#

Top level is a bit tricky because of naming shenanigans

#

i.e. DISALLOWED_HEADERS at top level is ambiguous

#

and similarly is CLIENT_DISALLOWED_HEADERS

#

Having it in the client sub-module is fine though

wild marlin
#

We should not going full syn

silent marsh
#

I wonder if build.rs shenanigans can detect a nightly compiler nicely

#

I'm not really a fan of the nightly feature flag in tokio-websockets, but having the nightly-only functionality enabled is a massive gain vs not having it

wild marlin
#

tbh I am not a big fan of upgrading features without being explicit.

#

There is apperently also a CFG_RELEASE_CHANNEL that is set so it can be read from build.rs

#

I feel like setting nightly automatically can give some really funky errors.

silent marsh
#

There's no downside to enabling nightly flags on a nightly compiler other than breakage when the feature flags get renamed in rustc and the crate hasn't been updated yet, and I'd argue that's part of what you opt in for when using a nightly compiler

wild marlin
#

Tbh that is exactly the issue I think about, where I used to work we were stuck for some reason on a year old nightly release, that would more or less bar us from using it with no way to opt our other than a fork.

silent marsh
wild marlin
wild marlin
#

And again

silent marsh
#

the fuzzer does not seem very happy

#

==2605== ERROR: libFuzzer: deadly signal

#

oh nvm

# 
thread '<unnamed>' panicked at fuzz_targets/stream.rs:225:51:
invalid close code [136, 130, 218, 36, 131, 192, 217, 201]

this is the actual error

#

Ah, yeah, this is a bit annoying

#

i think the fuzzer needs to verify that the closecode it creates is sendable

#

ughhhhh is_sendable is private

silent marsh
#

@wild marlin should I start updating the book already?

wild marlin
#

Sure, I will probably sit down and do everything for the release tomorrow

wild marlin
#

@karmic aurora Have you been running bots with zstd?

karmic aurora
#

I've done hello and hearbeat exchanges but I've not done reconnects for example

wild marlin
#

I thought that maybe we should wait with replacing zlib until we have run it for a while.

#

How much work do you think it would be to support both for 0.16

#

(I have moved @frank escarp over to it so I will keep an eye on it)

#

I don't think it is missing what you wrote about since we already have a entry for that endpoint for the put route.

#

@karmic aurora I think zstd is possible the last thing in the release, but if decided to add it as a option instead of replacing it I will just cut a release now.

primal torrent
#

Github isn't great on mobile

wild marlin
#

@karmic aurora Are you okay with going forward without the zstd pr for now?

karmic aurora
#

I think many others would dislike that if there's a large delay for the next point release (I also dislike adding even more features, but that's another thing)

#

but otherwise yes

wild marlin
#

I think I will make a point release as soon as we have added zstd just to keep a bit on top of it.

wild marlin
silent marsh
stoic umbra
#

I noticed that the CreateResponse does not follow the Client's default_allowed_mentions

#

I had a look at twilight code but I'm not sure what the best way to fix it is

wild marlin
#

hmm that should probably be fixed.

wild marlin
#

Been a bit busy with work this week, but I should be able to finish it up this weekend.

wild marlin
#

@silent marsh @karmic aurora Any opinion of MSVR?

wild marlin
#

Also I have updated the changelog

wild marlin
jaunty jewel
#

Do we need any features released between 1.80 and 1.84?

wild marlin
#

Probably not, but I am not sure I want to be careful not to use any of the new features.

jaunty jewel
#

I see what you mean

#

Lemme see what's new

wild marlin
#

Like we say MSRV is 1.84, but it may work with earlier, then we can say "please update :)"

jaunty jewel
#

i guess so, but we arent gonna be using rust-version

silent marsh
#

That's what tokio-websockets has and what twilight currently has as well, and I don't see us needing any of the new features since then.

#

But I generally don't mind a newer MSRV if someone can make a case for it

wild marlin
#

Tbh I think we should change so we are okay with upping the msvr on minor releases, but it is not something I will really go after tbh

silent marsh
#

now that cargo has an msrv aware resolver, sure

#

changelogs and book changes look very good, seems like we need another clippy fix for 1.84

silent marsh
#

@wild marlin by the way, are you going to RustNL?

#

I spent a lot of time chatting with Ralf Jung at 38c3 and he said I should definitely go there

wild marlin
#

Yeah I am, we will see if any of the talks I proposed will be accepted

#

Or well Rust week as they call it now

silent marsh
#

Cool, I saw tickets are cheap so I guess I'll go as well

#

Just need to sort out some hostel or so

wild marlin
#

I think tomorrow is the last day for early bird

#

You are still a student right

image.png
silent marsh
#

Yep

#

I'll also be at FOSDEM this year in case someone wants to say hi

wild marlin
#

I am probably not there, but you should go say hello to the Futhark people

silent marsh
#

Don't know how much time I'll have for talks yet

#

I'm mainly there as part of my involvement in PostmarketOS, which means I'll spend a lot of time at our booth and the talks in the devroom

#

I should make a personal schedule

silent marsh
#

I can PR some README changes for it later

#

later as in ~30mins

wild marlin
#

👍

#

I will do releases first thing tomorrow if nothing have come in before then.

silent marsh
#

I'll also do a clippy PR then

#

Will also look at #2407

#

By the way, thoughts on dropping lavalink for 0.16 and introducing it back later?

wild marlin
#

Its broken anyway right?

silent marsh
#

In an ideal world I'd wanna update it and remove it from twilight entirely because it's not a Discord API

silent marsh
wild marlin
#

And there is a practical Rust alternative in Songbird now as well

#

I already have a draft pr with the 0.16 changes there

silent marsh
#

There's an open PR to improve this that I said I'd take over, which I haven't gotten around to yet

#

I think we should drop lavalink from releases, I'll finish the PR and then we can orphan it and ask interested users if they'd want to maintain it

silent marsh
wild marlin
#

Is there?

silent marsh
#

Kind of, there's a symphonia fork with the necessary changes which hasn't been merged back yet

#

Language barrier seems to hinder it

wild marlin
#

Ah

#

Songbird still relies on libopus a bit

silent marsh
#

Getting rid of all C deps in songbird would be awesome

#

But yeah, what's your thoughts on this?

wild marlin
#

Yeah I think we should do it since it is in such a bad shape and none of us are interested in working on it much.

#

Could we split it out in its own repo?

silent marsh
#

That would be the first step for orphaning it in my view, yes

wild marlin
#

Are you git mage enough to remember how that is done?

silent marsh
#

Is that even possible while retaining history?

#

The commits in the lavalink directory often also touched other ones, I doubt this is possible

wild marlin
silent marsh
#

Oh, interesting

#

I don't think we should move it out of the monorepo until I finish up that PR, otherwise the PR is lost

#

We could also push the branch from the PR to the new repo and re-open it though

wild marlin
#

Okay will just have to do a bit of shell magic to skip it in the release then.

silent marsh
#

Clippy has some serious issues rn, lol

# 
warning: this looks like a formatting argument but it is not part of a formatting macro
 --> /home/jens/twilight/clippy.toml:2:8
  |
2 |   "AutoMod",
  |        ^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#literal_string_with_formatting_args
  = note: `#[warn(clippy::literal_string_with_formatting_args)]` on by default
jaunty jewel
#

lol

wild marlin
#

Can I get some quick reviews for 2408 and 2409?

jaunty jewel
#

I'll go ahead and merge 2409 first

wild marlin
#

Okay I am going to cross fingers now and hopefully not blow everything up.

#

Its going

wild marlin
#

Well that was an annoying default git commit message

#

otherwise I think it went alright

#

had to do a bit of on-the-fly editing because I had to do the releases in the correct order

#

All the tags should be there, they just did not arrive in #github-log

wild marlin
#

Okay I think I am actually done with the release now

jaunty jewel
silent marsh
#

Woohoo, this is a pleasant surprise to wake up to

wild marlin
#

Oh yeah, missed that comment

karmic aurora
#

My hope with gateway-queue#26 is that it lowers the barrier for users to contribute to the "refill" logic.

wild marlin
#

Hmm I should probably try and run tests.

gaunt pollen
#

Test in production hehe

wild marlin
#

I was getting to the station as I ran git push the final time, it was a fight against time

wild marlin
#

when you make a typo in the commit message of a commit that fixes a typo

#

Though I might just force it through because it is stupid.

wild marlin
#

@karmic aurora ✅

eternal owl
wild marlin
karmic aurora
#

Pushed some very WIP http rate limiter bucket & global limit support to vilgotf/new-limiter

karmic aurora
eternal owl
gaunt pollen
#

Could it be that the inmemory cache forgets to update the user at MemberUpdate events?

wild marlin
#

Broke out ratelimit discussions into #1336259117731414016

wild marlin
#

If anyone have some time this weekend for the couple of open pr's we have i can make a point release with the various smaller things

silent marsh
#

Got a list of the ones you want me to look at? I have some time today

#

Otherwise my time is very limited in the next week :/

wild marlin
#

2400 would be nice.
2401, 2407, 2413, 2415 and 2417 are all pretty small changes.

#

It is pretty much the most recently opened ones except the ratelimit rewrite.

silent marsh
#

Will look over those in the early evening

#

I'll also try to remember to create a MR for the MSRV change in the book

wild marlin
#

Would be nice as well

silent marsh
#

Done, if there are any other urgent ones let me know

wild marlin
#

Hmm @karmic aurora does it make sense to try and send a close frame on a zombied connection?

silent marsh
karmic aurora
# wild marlin Hmm <@131517556786855937> does it make sense to try and send a close frame on a ...

Discord explicitly tells us to cleanly close the WS connection with a close code, so I wouldn't dare change that without them saying otherwise.
From https://discord.com/developers/docs/events/gateway#heartbeat-interval-example-heartbeat-ack:

If a client does not receive a heartbeat ACK between its attempts at sending heartbeats, this may be due to a failed or "zombied" connection. The client should immediately terminate the connection with any close code besides 1000 or 1001, then reconnect and attempt to Resume.

karmic aurora
wild marlin
#

I am apperently blind

#

Hmm I wonder what causes it to get stuck then, do we have any timeouts for the websocket communication?

#

I guess this is also a bit special because I don't think I have ever seen it on my bot so it might be dependent on traffic a lot.

karmic aurora
#

What I don't understand (assuming the issue's not in tokio-websockets/twilight) is that the WS connection is managed by Cloudflare, which should not be impacted by the gateway failing

wild marlin
#

Like the closest I can find in the rfc in section 7.2.1 is

7.2.1. Client-Initiated Closure

Certain algorithms, in particular during the opening handshake,
require the client to Fail the WebSocket Connection. To do so, the
client MUST Fail the WebSocket Connection as defined in
Section 7.1.7.

If at any point the underlying transport layer connection is
unexpectedly lost, the client MUST Fail the WebSocket Connection.

Except as indicated above or as specified by the application layer
(e.g., a script using the WebSocket API), clients SHOULD NOT close
the connection.

#

So the question here is what exactly does the second section mean, and can a timeout go under that.

#

Also section 7.1.1 seems to say that a client initiated TCP close is allowed in some cases

#

In abnormal cases (such as not having received a TCP Close from the server after a reasonable amount of time) a client MAY initiate the TCP Close.

karmic aurora
#

It's not clear that the transport layer (TCP) is necessarily failing in this case though

wild marlin
#

Yeah but if we don't receive anything after we send a close for it for a reasonable amount of time we can also close it.

karmic aurora
#

But I misremembered that there are some exceptions for not awaiting an echo close message given in 7.1.7

wild marlin
#

Yeah seems like that can cover it as well.

karmic aurora
#

Anyway, I'm not too anal about strict spec compliance, but more so that I would like to know the actual failure point so that we can fix that instead of adding a bandaid. Though I should probably comment such a bandaid to that issue thread for anyone affected

wild marlin
#

Yeah I would also like to know, because it only seems to happen for some people.

#

Linux seems by default to kill a tcp connection after 60 seconds.

karmic aurora
#

that sounds incompatible with, for example, long polling

wild marlin
#

The 60 seconds seem to be if it has not received a response after sending a request so it will still work with long polling.

#

Hmm i just quickly tried to kill my internet connection after having started my bot and it never seems to recover

karmic aurora
#

That nice (as in we can troubleshoot that)

wild marlin
#

It seems to only happen if the connection is down for a while

#

At least a while after it has sent a heartbeat ortherwise it works out

#

I don't get the zombied message though.

#

And nothing in either debug or trace related to it.

karmic aurora
jaunty jewel
#

I went ahead and wrote a tentative form of the voice channel status thing

#

well seems like I also need to change cache trash

karmic aurora
wild marlin
#

hmm that is odd

#

I did it for around one-two missed heartbeats

#

How do you turn off the internet?

#

just plugging out the cable / turning off wifi?

#

How is it you enable the tokio trace feature?

#

Here it was

karmic aurora
#

Tokio doesn't instrument IO, so it's not useful in this case

karmic aurora
wild marlin
#

Hmm maybe I will try and connect it up to tokio-console

wild marlin
karmic aurora
#

What does the shard log if it doesn't zombie for you? Does it log multiple sending heartbeats in a row, received heartbeat ack even while disconnected, or does it log nothing at all?

wild marlin
#

I get one sending heartbeat and then nothing more

karmic aurora
#

I get sending heartbeat followed by zombied when disconnected (the heartbeat is never sent, but it's buffered to the kernel I guess)

wild marlin
#

I never got the zombied

karmic aurora
#

That's really interesting, something is calling Shard::disconnect then for you (as that's what disables the heartbeat interval, which in due time detects the connection as zombied). Could you add some logging and try to figure out what's calling it?

karmic aurora
wild marlin
#

@karmic aurora Tried with tokio console and it is just as strange, things just stopped getting polled:

image.png
#

I think the "running" means that it is still going, the task there is the shard task

#

So it seems like it is getting stuck?

#

Yes it seems that task gets stuck after a while

#

It willl just have the play symbol and begin to count up as busy

#

I think the solution might be to have some kind of timeout, probably in our code.

wild marlin
karmic aurora
wild marlin
#

And it ends up giving a none

#

but I never get the debug log in that branch so I think poll_close never finishes

#

@silent marsh ^This seems to never complete if the connection is dead in some specific way

silent marsh
#

can you create a minimal reproducer?

wild marlin
#

I am not sure, but I will try and toss some more logs into tw

#

@silent marsh It filled my whole terminal buffer with logs blobsved

#

About a billion of these messages:

[/Users/erk/dev/tokio-websockets/src/proto/stream.rs:426:21] bla = Err(
    Io(
        Custom {
            kind: Other,
            error: Error {
                code: -9806,
                message: "connection closed via error",
            },
        },
    ),
)
silent marsh
wild marlin
# 
while let Some(bla) = ready!(self.as_mut().poll_next(cx)) {
  let _ = dbg!(bla);
}
silent marsh
#

ohhhhh

#

this is an easy fix

#

is_some_and(Result::is_ok)

#

that should fix it

wild marlin
#

@karmic aurora Just tested the proposed fix and it solves at least this zombie connection

silent marsh
#

I'd like a regression test. This should be fairly easy to reproduce if you write a server that aborts upon receiving a close message

#

If it aborts immediately, the response to the close frame can't be sent so it should trigger the infinite loop

karmic aurora
#

Very nice work @wild marlin

wild marlin
# silent marsh I'd like a regression test. This should be fairly easy to reproduce if you write...

This does not seem to work, and I am not quite sure if that is what you are thinking of:

#![cfg(feature = "server")]
use std::{net::SocketAddr, time::Duration};

use futures_util::SinkExt;
use tokio::net::TcpListener;
use tokio_websockets::ServerBuilder;

const PORT: u16 = 3001;

#[tokio::test]
async fn test_pr102_regression() {
    let addr = SocketAddr::from(([127, 0, 0, 1], PORT));
    let listener = TcpListener::bind(addr).await.unwrap();

    let bla = tokio::spawn(async {
        let (mut ws, _) =
            tokio_websockets::ClientBuilder::from_uri("ws://127.0.0.1:3001".parse().unwrap())
                .connect()
                .await
                .unwrap();
        tokio::time::sleep(Duration::from_secs(1)).await;
        ws.close().await.unwrap();
    });

    let (conn, _) = listener.accept().await.unwrap();
    let (_request, mut server) = ServerBuilder::new().accept(conn).await.unwrap();
    server.close().await.unwrap();
    drop(server);

    bla.await.unwrap();
}
silent marsh
#

let me try

wild marlin
#

you can just push to the pr if you make something that works, I will probably not have a lot of time tomorrow.

silent marsh
#

no worries, thanks for digging into this

silent marsh
#

hmm my idea for a reproducer does not work, shame

wild marlin
#

Yeah I am not quite sure because it did not happen in all cases for everyone, I don't think Vilgot could reproduce it

silent marsh
#

It would be a shame to merge this without a regression test

#

so what exactly was the connection state and which poll_close call in twilight triggered this?

#

i am missing some important context

wild marlin
#

I am not sure because my terminal just got filled up, let me check again

#

Its Active

karmic aurora
#

I commented some thoughts on the PR

karmic aurora
# wild marlin Its `Active`

I think Adrian is asking about the TWS StreamState and not ShardState. Both have a variant called Active. Want to make sure we're not talking past each other

wild marlin
#

@karmic aurora You and Adrian can probably just take it over will probably not have time before sunday

wild marlin
#

The first error was actually a timeout:

[/Users/erk/dev/twilight/twilight-gateway/src/shard.rs:917:13] &bla = None
[/Users/erk/dev/tokio-websockets/src/proto/stream.rs:418:9] &self.state = Active
[/Users/erk/dev/tokio-websockets/src/proto/stream.rs:423:13] &bla = Err(
    Io(
        Os {
            code: 60,
            kind: TimedOut,
            message: "Operation timed out",
        },
    ),
)
silent marsh
karmic aurora
#

I guess it's getting stuck on L191, as its got pending data and that poll_flush returns Poll::Ready(Err())

silent marsh
#

re Vilgot: I believe the behaviour of poll_next as-is is fine. You should not attempt to read from a stream after it has errored once

#

As far as that goes, Erk's patch is correct

silent marsh
#

In that case changing the state to CloseAcknowledged upon errors is fine

#

And probably the prettier way

# 
#![cfg(all(feature = "client", feature = "server"))]
use futures_util::SinkExt;
use tokio::net::TcpListener;
use tokio_websockets::{ClientBuilder, Message, ServerBuilder};

#[tokio::test]
async fn test_pr102_regression() {
    let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
    let addr = listener.local_addr().unwrap();

    let server = tokio::spawn(async move {
        let (mut stream, _) = listener.accept().await.unwrap();
        let (_, mut ws) = ServerBuilder::new().accept(&mut stream).await.unwrap();
        drop(stream);
        println!("closed");
    });

    let (mut client, _) = ClientBuilder::new()
        .uri(&format!("ws://{addr}"))
        .unwrap()
        .connect()
        .await
        .unwrap();
    assert!(client.send(Message::text("hello!")).await.is_err());
    client.close().await.unwrap();

    server.await.unwrap();
}
#

this confuses me

#

the tcp stream should be closed, and according to the logs it gets closed, but the send still succeeds for w/e reason

# 
running 1 test
closed
[src/proto/stream.rs:378:31] poll_write_buf(Pin::new(io), cx, &mut buf) = Ready(
    Ok(
        12,
    ),
)
[src/proto/stream.rs:403:9] "flushing" = "flushing"

thread 'test_pr102_regression' panicked at tests/pr102_regression.rs:32:5:
assertion failed: client.send(Message::text("hello!")).await.is_err()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test test_pr102_regression ... FAILED
silent marsh
#

yeah i think this is why i cannot reproduce, lol

#

ah! i can reproduce with DuplexStream

#

finally

# 
#![cfg(all(feature = "client", feature = "server"))]
use std::time::Duration;

use futures_util::{SinkExt, StreamExt};
use tokio::{io::duplex, time::timeout};
use tokio_websockets::{ClientBuilder, Message, ServerBuilder};

#[tokio::test]
async fn test_pr102_regression() {
    let (tx, rx) = duplex(64);

    // Create a server that sends a close message and immediately closes the
    // connection
    let mut server = ServerBuilder::new().serve(rx);
    // Don't use ws.close() here to avoid a graceful handshake
    server.send(Message::close(None, "")).await.unwrap();
    // This will close the connection
    drop(server);

    // Create a client and make sure the close frame from the server was sent and
    // the connection closed
    let mut client = ClientBuilder::new().take_over(tx);

    // Receive the close frame, this will queue the close ack frame
    assert!(client.next().await.unwrap().unwrap().is_close());
    // The close ack frame is queued, the connection is closed, before PR 102 this
    // would trigger an infinite loop in close()
    // For the regression test, fail the test if this doesn't close within a second
    assert!(timeout(Duration::from_secs(1), client.close())
        .await
        .is_ok());
}
#

i would prefer if vilgot could push his changes to erk's branch instead, i prefer them

wild marlin
#

@karmic aurora Can you pop the test Adrian made in there as well?

wild marlin
#

@karmic aurora the docs say somewhere that if we get such a null heartbeat we have to heartbeat immediately

karmic aurora
#

Yes and that's what we are doing, but we fail after poll_next when deserializing

wild marlin
#

Oh that's fun vikingblobsved

jaunty jewel
silent marsh
#

Published version 0.11.3, all should be fine now

#

Sorry for taking so long, I was a bit busy recently

wild marlin
wild marlin
#

Anyone up for helping with the "Components V2" stuff?

#

I can probably finish it in time for the release anyway, but it would be nice with some more eyes on it

#

I think the release they are aiming for is the 11th of march

opal juniper
wild marlin
#

And thanks!

opal juniper
#

No problem

wild marlin
#

Let me know if you need something tested as well

silent marsh
opal juniper
karmic aurora
#

MSRV job broke again...

silent marsh
#

We should just do what I do in tokio-websockets

#

Rather this than pin individual packages manually, that's gonna be hell to keep in sync

karmic aurora
#

Nice, I did that in my PR but I can split it out into its own

#

There's one not quite like the others 🤔

image.png
wild marlin
#

Feature combinations is a bit of a mess tbh, it takes ages to check

wild marlin
#

The components v2 stuff got delayed a bit so it is there is not as much pressure to finish it up.

wild marlin
#

@silent marsh Want to take that issue?

silent marsh
silent marsh
#

I realized we weren't even on hyper v1 yet, that should do it :)

karmic aurora
#

Can I get a couple of quick reviews on #2426 (clippy lints)

wild marlin
#

@karmic aurora That is a lot of false positives?

karmic aurora
wild marlin
karmic aurora
#

That requires 1.81, but we're on 1.79 atm

#

We should definately switch all allows to expect once we bump that

karmic aurora
#

for twilight-model

#

was unable to reproduce locally

#

nvm, I can reproduce it on nightly

wild marlin
#

Nice

lone current
#

Oh yeah been seeing ICEs for the last 2 weeks on nightly

silent marsh
#

We may want to use flate2's zlib-rs backend these days, it's faster than the zlib one and pure Rust

#

Actually they say that zlib-rs outperforms all the C backend so perhaps we want to get rid of zlib-ng as well

#

hmm nevermind, we deprecated zlib already

wild marlin
#

We might want to not deprecate it, at least not in next release.

wild marlin
silent marsh
wild marlin
#

Yeah zlib-ng

silent marsh
#

cool though

wild marlin
#

There is some benchmarks in that pr

silent marsh
#

holy hell

wild marlin
#

its fast

silent marsh
#

for that alone we probably want to keep zlib-ng support

#

zstd is probably nowhere near close

wild marlin
#

I don't know if I have time today to go though and see if anything has changed, I am not sure what else we still need.

fluid dust
#

validation, documentation, and some serialization changes

#

there are some nullable fields twilight doesn't deser properly

wild marlin
#

I guess I am bad at reading typescript definitions since that was the only thing we had.

fluid dust
#

nah not your fault

#

his definitions only declare the presence of fields, no nullability lol

eternal owl
#

@wild marlin I appear to be the only direct owner of twilight-validate, so I've invited you to also own it.

wild marlin
lone current
#

I’ve moved most of the commands to a web panel

wild marlin
#

Yeah it makes garfield look odd as well so I don't think I will use it either, so I just have to find some time and will to finish it up

jaunty jewel
#

@wild marlin do you need any help moving components v2 forward? I might as well look at it soon

jaunty jewel
#

Hmmm seems like MSRV bumped for some dependencies

wild marlin
#

I will have a look at it

wild marlin
#

I have pushed a load of comments and serialization fixes.

#

The main thing missing now is getting the builders updated and documented and adding components_v2 to validate

#

I have also rebased it now

jaunty jewel
#

alright

jaunty jewel
#

@wild marlin I suppose we need some tests for the individual builders like the other ones twilight-util?

silent marsh
#

@karmic aurora I think we can bump MSRV and use edition 2024

#

Now that our MSRV policy allows this, we should just go for it

wild marlin
silent marsh
jaunty jewel
#

@wild marlin could you look at my action row test, I wanna know whether that is sufficient

jaunty jewel
#

@wild marlin should the component enum implement From for all of the variants?

wild marlin
#

The tests looks fine imo

jaunty jewel
#

I'll work on those now

jaunty jewel
jaunty jewel
#

I doubt I have ever touched validation and know enough about components v2 to implement that part correctly

wild marlin
#

tbh I don't know enough either, but I guess we will have to learn more about it that we really want to

jaunty jewel
jaunty jewel
#

the links in the old validation docs are outdated

# 
let function = if is_v2 {
    crate::component::component_v2
} else {
    crate::component::component_v1
};
for (idx, component) in components.iter().enumerate() {
    function(component).map_err(|source| {
        let (kind, source) = source.into_parts();

        MessageValidationError {
            kind: MessageValidationErrorType::ComponentInvalid { idx, kind },
            source,
        }
    })?;
}
#

uh this seems cursed

#

💀

jaunty jewel
#

@wild marlin I wonder how to detect components v2 when updating messages

wild marlin
jaunty jewel
wild marlin
#

Ah yeah right, maybe just leave a todo then

#

Is the components fully constructed then we could check those

jaunty jewel
#

yeah

#

but we can't be sure whether its v1 or v2

#

¯_(ツ)_/¯

wild marlin
#

Are the components also different?

#

Seems like it

#

Maybe we should just drop validation there and leave a note?

jaunty jewel
#

V2 has a LOT more components

jaunty jewel
#

🤔

#

(Yeah I planned on just dropping validation for editing messages by always using v2 validation)

wild marlin
# 
/// Validation of components is not done automatically here, as we don't know which component version is in use, you can validate them manually using the [`twilight_validate::component`] function.
#

Something like that

jaunty jewel
#

it would be great if someone could test whether IS_COMPONENTS_V2 is allowed in creating a forum thread (for the first message of given thread)

image.png
wild marlin
#

I am pretty sure it is, it will just look odd in the overview

jaunty jewel
#

The documentation for flags does seem to suggest otherwise

wild marlin
#

I can check it out later

jaunty jewel
#

Great

#

Thank you very much

#

I'll just update all the HTTP routes in question first and then go to the uh

#

actual validation of components

#

its better for me to do those first cuz IS_COMPONENTS_V2 is also supported so the docs needs to be updated

jaunty jewel
#

@wild marlin I now have a primitive implementation

#

as for testing I delegate it to sometime later or someone else

jaunty jewel
#

@wild marlin what have you discovered?

wild marlin
#

@jaunty jewel It works

#

(It says "Click to see message" in the first image)

# 
    client
        .create_forum_thread(Id::new(123), "V2 Test")
        .message()
        .flags(MessageFlags::IS_COMPONENTS_V2)
        .components(&[Component::TextDisplay(TextDisplay {
            id: None,
            content: "This is a TextDisplay".to_owned(),
        })])
        .await
        .unwrap();
jaunty jewel
#

I’ll update when I get home

wild marlin
jaunty jewel
#

That’d do also

#

Thanks

wild marlin
#

I have pushed it together with some component counting.

jaunty jewel
#

but I'll look into this later

wild marlin
wild marlin
#

I think I will say that the PR is ready for review

primal torrent
#

Ill try to make some time for it this weekend

jaunty jewel
#

Looking forward to 2422 and 2429 landing

jaunty jewel
#

one more review is needed for 2422 to land

jaunty jewel
#

How often to we synchronize next with main?

jaunty jewel
karmic aurora
#

Approved whilst on semester dealwithfluts

jaunty jewel
#

Amazing

jaunty jewel
#

seems like 0.16 date ought to be updated

wild marlin
wild marlin
jaunty jewel
#

2024-??-??

#

also, if I may ask, 0.17 when?

wild marlin
#

Probably soon

#

There are a couple of prs I want into it before

jaunty jewel
#

well, obviously #2422

eternal owl
#

i want to transfer the server. who should i transfer it to, erk? because it's verified this will be a customer support request

jaunty jewel
silent marsh
#

i think erk would be the best option, yes

wild marlin
#

Sure

eternal owl
#

okay

jaunty jewel
eternal owl
#

why do you have to make a separate account for discord support

wild marlin
#

A guess would be that it then can be used to regain access of lost accounts

swift gull
#

Do they provide support using a different system once you lose access to your support account? kappa

jaunty jewel
jaunty jewel
#

cc @karmic aurora

jaunty jewel
#

👍

karmic aurora
#

#2424 (retry on 429) ready for review

jaunty jewel
#

#2422 (components V2) awaiting one more review

jaunty jewel
#

ok #2438 will be targeting next instead or I deprecate GUILD_EMOJIS_AND_STICKERS and add a new GUILD_EXPRESSIONS

jaunty jewel
#

should soundboard sounds be cached in cache-inmemory?

#

(actually i think that should be a yes)

jaunty jewel
#

seems like we are quite behind the discord API rn

eager birch
#

yea..

jaunty jewel
#

anyway I’ll try to get soundboards done asap

wild marlin
jaunty jewel
#

which is only sent if the bot requests it

#

with a REQUEST_SOUNDBOARD_SOUNDS command

wild marlin
#

Tbh I am not sure if we should cache them then

#

If it does not come automatically I don't think it is worth it

jaunty jewel
#

Right

#

Makes sense

eternal owl
#

presences also don't come automatically unless you request them. how would you reconcile caching presences but not soundboard sounds?

wild marlin
#

So maybe it should be cached.

primal torrent
eternal owl
primal torrent
#

huh missed that

#

prob best to cache them as well then, should just have a docs warning to do the initial request

jaunty jewel
#

well I missed that

jaunty jewel
jaunty jewel
jaunty jewel
wild marlin
#

Its a bit odd that they have added a top level endpoint for that though

jaunty jewel
#

But... what can we say?

jaunty jewel
#

Anyway I'll put the rest of the endpoints in guild module then

jaunty jewel
#

Would be great if we could get this quick fix for twilight-validate to be merged and stop build docs from failing

karmic aurora
#

Thanks for being so quick @jaunty jewel

jaunty jewel
#

#2439 needs one more review

primal torrent
#

Ill try to do another round of reviews this weekend

jaunty jewel
#

2422 and 2439 requires 1 more review

wild marlin
#

We should make a wrapper for Option<Option<T>>

wild marlin
jaunty jewel
wild marlin
jaunty jewel
eternal owl
#

I remember considering this long ago and it was kind of just annoying to model and convert user provided data to the abstraction

wild marlin
#

@karmic aurora Should self.connection not be set to None in a normal disconnect and not only in abnormal closures?

#

Otherwise I don't think it ever attempts to reconnect?

wild marlin
silent marsh
#

sure, will do

#

I'm not a huge fan of that PR as is, I'd prefer reworking the current file a bit

#

I have a local draft from a few years ago that I should continue

#

We can get rid of the cross compilation logic to simplify it a lot since there's native arm64 runners now

wild marlin
#

Ahh yeah that would be nice

#

Tbh the only thing I don't really like in the original is the use of musl.cc

silent marsh
#

But it won't be necessary anymore if we only do native builds

#

I'll put up a PR with what I have in mind

karmic aurora
jaunty jewel
#

@wild marlin should Component also implement From<*Builder>

#

the asterisk I mean the component builders

wild marlin
#

I am not sure tbh

#

Rebased the branch and added label to the button builder

wild marlin
#

@karmic aurora A thing I dislike a bit is the very large match statements in futures, there is also one in the shard, but I think we should refactor those out in some way at some point.

karmic aurora
#

yeah I get you

karmic aurora
#

Re: #general message
Do we

  1. Not handle this
  2. Create a new crate
  3. Extend gateway-queue
#

This rate limit is unique in that you are not punished for exceeding it

#

Combined with it RATELIMITED being a dispatch event leads me to believe that Discord may not bots to explicitly handle

#

But most important is the user POV: is a bot that enqueues exceedingly many events bug-free? I would argue that it would be a bug in a bot regardless of whether we space out actually sending them to Discord via a new rate limiter

#

Then, would adding a ratelimiter to apply a band-aid for buggy bot behavior be worth it?

valid oracle
#

I personally don't think rate limiting it on the library side is a good idea because this is such a rare and exceptional limit to hit that the bot developer should definitely know about hitting

wild marlin
#

Yeah I don't think we should handle that rate limit either, tbh I feel like you are doing something wrong if you are hitting that at all.

primal torrent
#

might be worth adding a warning to the docs though to the request

wild marlin
#

Yeah adding it to the docs and of course implementing the new event with appropriate docs

jaunty jewel
#

When can we get components v2 merged?

jaunty jewel
#

@wild marlin so these should be Option<Option<T>>?

image.png
#

Some(None) -> null
None -> field not present

wild marlin
#

Yes

jaunty jewel
#

idk

#

because we have “skip serialisation if” tho

wild marlin
#

No that is fine

#

It only looks at the outer option

jaunty jewel
#

right

silent marsh
#

I will take a look at components v2 tonight, let's try to get this in soon

wild marlin
#

I am away on holiday until next week, so I will probably not have too much time to answer stuff until tuesday

jaunty jewel
#

I’ll be available to answer questions if any

jaunty jewel
#

one more review to get this merged quickly

silent marsh
#

Components v2 PR mostly needs cosmetic fixes and some work on the validation side

#

I sent a review, once that's addressed LGTM

wild marlin
#

I think we should publish 0.17 as soon as possible after the merge to get it and the new rate limiter out, though it would be nice to have tokio-websocket timeout in it as well

silent marsh
#

imho we should probably just add the timeout in twilight-gateway and not tws

#

this is a perfect place to add tokio::time::timeout around the connect call

wild marlin
#

Yeah that should work as well

#

Do you want to do that as well?

silent marsh
#

Yeah I'm working on it right now

wild marlin
wild marlin
#

@silent marsh I will just merge your http-proxy pr

silent marsh
#

Nice

wild marlin
#

Also nice there was a security issue in slab that is newer than the pr

primal torrent
#

Has the new rate limited been tested at scale? If not a rc moght be best first for a week or 2

silent marsh
#

I'm fairly sure that there are plenty of people using the git version because of the fixes and features present in main

karmic aurora
#

What should be done with the (deprecated) zlib compression support? Adrian discussed retaining it due to some arch's hardware acceleration

#

I'm pro deleting it cause it's straight up worse for 99% of use cases

wild marlin
#

Tbh I think we should let it stay if people need to cross compile, since zlib exists in a pure rust version now

silent marsh
#

Yeah zlib-rs is great and I think giving users the option to have a fast, pure-Rust implementation of compression is very nice to have

#

Though we still need to change the feature flags in flate2 to use zlib-rs iirc

wild marlin
#

That is the default now, but maybe we are being a bit too smart with feature flags

silent marsh
#

The Trifecta Tech Foundation says

Development of zstd began in July 2025, with the first release of the decoder planned for December 2025.

wild marlin
#

Ooh, that is very nice to hear

silent marsh
wild marlin
#

Would love for a safe implementation to exist

silent marsh
#

BTW I'll PR a tokio-websockets 0.12 update to twilight today

#

that and components v2 is what I want before a new patch release

silent marsh
#

since tokio-websockets 0.12 enforces a global rustls crypto provider which we shouldn't be setting in twilight, but rather the user should be doing it

#

i would say that for now we can check if one is set and if not, we set the global to whatever is configured in crate features

#

but for twilight 0.17 we should also require a global crypto provider

silent marsh
#

okay, seems like we need an MSRV bump for edition 2024 to update simd-json

jaunty jewel
#

2024 should be ok enough now

silent marsh
#

i'm very glad that we specifically allowed msrv changes in patch releases with 0.16

jaunty jewel
#

lol

karmic aurora
silent marsh
#

doesn't seem to work for me

swift gull
#

IIRC, there are a lot of things that (silently) cause rustfmt to break while formatting doc comments.
Does it display anything with error_on_line_overflow or error_on_unformatted?

wild marlin
#

Or wait that is a diffferent error

wild marlin
#

Hmm m #github-log

silent marsh
#

oh dear

#

i'm not sure how much about my original review i even remember now

wild marlin
#

@silent marsh / @karmic aurora are you okay that I just do a merge commit and merge the two commits from main into next?

karmic aurora
#

yes 👍

wild marlin
#

its only a few commits ahead but I don't think I want to do another feature release of 0.16.

silent marsh
#

sure

#

can do that in a few hours

south sphinx
#

@wild marlin thanks for the quick review. I can add role_mentions if you think it's important, though in my testing it's just not there:

{
    "t": "MESSAGE_CREATE",
    "s": 173,
    "op": 0,
    "d": {
        "type": 0,
        "tts": false,
        "timestamp": "2025-09-16T20:08:57.759000+00:00",
        "position": 0,
        "pinned": false,
        "nonce": "1417603136079527936",
        "message_snapshots": [
            {
                "message": {
                    "type": 0,
                    "timestamp": "2025-09-16T17:53:04.346000+00:00",
                    "mentions": [],
                    "flags": 0,
                    "embeds": [],
                    "edited_timestamp": null,
                    "content": "<@&734120922780532747> <@&559437170113511437>",
                    "components": [],
                    "attachments": []
                }
            }
        ],
wild marlin
#

Ahh only 11 months without a response.

#

So it is there if you use http apperently so I guess it still makes sense to add it

wild marlin
#

Also @karmic aurora do you have an outline of a algorithm for unknown paths in your mind?

karmic aurora
#

Yes and I came up with something incredibly simple (Endpoint replaces Path)

📎 path.rs
karmic aurora
#

All tests passed first try vikingblobsved

wild marlin
#

Assert that different (method, path) permutations result in different hashes.
Currently this is not true

#

Method is not taken into considaration

karmic aurora
#

It's used in the bucket map (HashMap<Entrypoint, Bucket>) via the derived hash impl

#

I think the algorithm should be:

  • If the bucket is known, use that (top-level resource + bucket) queue
  • Otherwise, fallback to a per-endpoint (path + method) queue
#

But I'm not convinced that is what's implemented

wild marlin
#

I was talking about:

    #[test]
    fn check_hash_uniqueness() {
        let state = RandomState::new();
        let mut hasher = state.build_hasher();

        let a = Endpoint {
            method: Method::Get,
            path: String::from("/guilds/bla"),
        };

        a.hash_components(&mut hasher);
        let a_hash = hasher.finish();


        let mut hasher = state.build_hasher();

        let b = Endpoint {
            method: Method::Post,
            path: String::from("/guilds/bla"),
        };

        b.hash_components(&mut hasher);
        let b_hash = hasher.finish();

        assert_ne!(a_hash, b_hash);
    }
#

But yeah that makes sense, I am not too sure that is what is implemented either, I will have a read through and see what I find.

karmic aurora
#

hash_components it meant to hash top-level components only ("bla" in this case)

#

hash_top_level_coponents was too long

#

Oh and I just realized that "guilds" needs to be included such that channels/1 and guilds/1 are not equal

wild marlin
#

Ahh that makes sense, maybe add a comment clarifying that

#

Should /applications not also be there

karmic aurora
#

That too

#

The top-level resource is a sort of bucket namespace

wild marlin
#

And /invites and /users, I will have a proper look later

#

Yeah

karmic aurora
wild marlin
#

Top-level resources are currently limited to channels (channel_id), guilds (guild_id), and webhooks (webhook_id or webhook_id + webhook_token).

#

Ahh

karmic aurora
#

Need to also update twilight-http to filter the out the query string from the path given to the ratelimiter

#

I don't really want any validation in the ratelimiter itself

wild marlin
#

You could just do:

Endpoint { method, path: http_request.uri().path().to_owned() },

since we already parse it into a URI in the request.

#

(Should note that then the path will always contain / so maybe strip that as well)

karmic aurora
#

The current implementation is more conservative, which results in fewer false positives (thinking we're not rate limited, when we in fact were), but sacrifices some concurrency

#

But this is not impacted by my PR, so I think we can shelve this discussion

karmic aurora
wild marlin
#

Looks pretty nice

wild marlin
#

Or would that lead to some other issue?

karmic aurora
#

What do you mean?

wild marlin
#

Or wait I think I misunderstood what you wrote

#

nvm

karmic aurora
#

All right

wild marlin
#

@karmic aurora is the TODO in your pr still relevant?

karmic aurora
#

Well I would still like to add tests, but I think the actual implementation is correct

eternal owl
#

@wild marlin I finally got around to transferring the server to you.

wild marlin
wild marlin
#

I think I will aim for merging components v2 this weekend @silent marsh if you have some time for a review it would be appriciated

silent marsh
#

I'll take a look right now

#

@wild marlin I think the documentation for validation still needs work, see my comment you marked as resolved

#

Once that's done I think this is good to go

wild marlin
#

Ah yeah, will try and write some docs

south sphinx
#

Is there anything I can be doing to help get my change merged in? It has one approval of two

wild marlin
#

I will have a look and see if I can do a bunch of merging this weekend

karmic aurora
#

Going to force merge the ci fix if no one objects

wild marlin
wild marlin
#

@silent marsh i fixed up the comments

wild marlin
#

Thanks to @opal juniper, @jaunty jewel, Kiranshila, notiku and @silent marsh we finally got one of the longest living PR's merged!

#

🎉

#

Oh I forgot a pr, blobsved

#

(actually I will probably wait until tomorrow)

silent marsh
#

:p

#

I'll rebase and update my PR today, sorry for not doing it

wild marlin
#

@south sphinx Just a heads up, you have used a email in the commits that are not connected to your GitHub account

slate oxide
south sphinx
wild marlin
#

Nah I just spotted it because it wanted to add you as a co-author to the pr that you are the author of

south sphinx
#

^^
I don't have my bot email on my github so that's probably why

wild marlin
#

That is the reason the commits look like this in the web-interface:

image.png
south sphinx
#

Thanks!

silent marsh
# 
error[E0063]: missing field `id` in initializer of `channel::message::component::button::Button`
   --> twilight-model/src/channel/message/snapshot.rs:181:62
    |
181 |                     components: Vec::from([Component::Button(Button {
    |                                                              ^^^^^^ missing `id`

error[E0063]: missing field `id` in initializer of `channel::message::component::action_row::ActionRow`
   --> twilight-model/src/channel/message/snapshot.rs:180:55
    |
180 |                 components: vec![Component::ActionRow(ActionRow {
    |                                                       ^^^^^^^^^ missing `id`
#

uhm

#

why do i catch this locally but our CI does not

karmic aurora
#

Cache issue?

karmic aurora
wild marlin
#

I will do another merge of main into next

wild marlin
karmic aurora
wild marlin