SpaceManiac / discord-rs

Rust library for the Discord chat client API
MIT License
385 stars 94 forks source link

Updated dependencies #143

Closed jD91mZM2 closed 4 years ago

jD91mZM2 commented 6 years ago

Don't merge this yet. I put it here so people can give ideas and contribute. This updates a few dependencies, but might not even be working. And there are still some TODOs.

~~Also, @SpaceManiac, what is the difference between inner_shutdown and raw_shutdown? After this pull request they're indentical...~~ Saw the difference. Will check out later

jD91mZM2 commented 6 years ago

Update: Confirmed not even starting a bot. Oh boy I have a lot more work to do...

EDIT: Oh, should have pointed out. I tested it by running the basic_chatbot example. That gave connect failed: WebSocket(ResponseError("Status code must be Switching Protocols")).

I think the problem is SSL, however, illegalprime says .split()ing an SSL stream is deprecated. Tried to use the feat/tokio branch, but for some reason Cargo didn't use it?!!

bdonlan commented 6 years ago

Regarding websocket .split() issue, a better approach might be to internally make use of the websocket async interface, at which point you can poll both the Sink and Stream interfaces simultaneously, and/or use the .split() call (which does work on a proper Stream + Sink). You would need to set up a tokio Core, but you could just pass the future for whatever synchronous action was triggered to Core.run() to run it just long enough to finish that action.

jD91mZM2 commented 6 years ago

I personally like the fact that discord-rs is a synchronous library. Apart from a few issues because of it, it's really nice to not have to use mutexes...

bdonlan commented 6 years ago

Tokio doesn't require any mutexes, though. It's perfectly suitable to being used entirely on a single thread, and has the type safety it needs to express when things aren't safe to migrate.

jD91mZM2 commented 6 years ago

That's really cool! Would this be a breaking change for the users?

EDIT: Currently searching for information about how to implement this. Help please :stuck_out_tongue_closed_eyes:
EDIT 2: This it? Seems like it would pretty much require an entire rewrite of discord-rs' connection handling... lol

tinaun commented 6 years ago

legolord208: yeah, thats why my impl of this is stalled too. I was actually going to add my branch up here as a wip for comments until i found yours.

basically i was thinking this: since we need to rewrite the whole connection logic anyway (splitting a ssl stream is unsound and always will be) - why should we keep trying to write our own event loops when theres async libraries that can provide them for us. this would also help with a lot of other issues around always-blocking connections, like #109

jD91mZM2 commented 6 years ago

Good idea. Wanna do the honors, or should I try? (Of rewriting)

tinaun commented 6 years ago

how well do you know tokio? ive done some stuff with it but im not an expert

jD91mZM2 commented 6 years ago

I have never used tokio in my life :stuck_out_tongue:

tinaun commented 6 years ago

ah ok. im gonna push a branch to my fork publically and work on it, and then you can look at it and work with it before its done

tinaun commented 6 years ago

a big problem is multipart requests, though. multipart doesn't support async hyper, and reqwest, providing a sync interface over async hyper, doesn't support them yet either.

jD91mZM2 commented 6 years ago

RIP. Let's keep that unimplemented! for now, and we'll figure something out later

tinaun commented 6 years ago

https://github.com/seanmonstar/reqwest/pull/160 so this looks like it's going to be merged in a couple of days, i'll merge it on my own branch until it is and a new release is pushed to crates.io.

Tahlwyn commented 5 years ago

Any news on this front? I'm getting all sorts of issues here relating to outdated dependencies and I don't really have the savvy with rust yet to update them myself.

jD91mZM2 commented 5 years ago

I'm afraid discord-rs is dead, or so I've heard. Guess serenity is the only option for discord libraries now

SpaceManiac commented 4 years ago

hyper/websocket/multipart dependencies have been updated elsewise