-
Notifications
You must be signed in to change notification settings - Fork 234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Promise v3 #1157
base: master
Are you sure you want to change the base?
Promise v3 #1157
Conversation
- changed ExtendedPromiseInterface to PromiseInterface - replaced Discord\Helpers\Deferred with React\Promise\Deferred - changed ->done(true function, false function) with ->then(true function)->catch(false function) - changed ->always() to ->finally() - added use React\Promise\Deferred where I think it was missing - updated $deferred->resolve() to $deferred->resolve(true) - updated $deferred->reject() to $deferred->reject(throwable)
@key2peace what is the state of this PR? Being stuck with v2 Promises (thanks, Composer, for not handling multiple versions of one package btw) means even something as banal as |
It wont be in v10, many dependencies still stuck with v2 promises about set_rejection_handle, Just be careful with your promise codes for now |
@SQKo DiscordPHP should at the very least handle the rejection path with a logger, with a log level >= warning. There is no reason to not do that since afterwards nobody could even catch the rejection currently. I think that's way better than leaving it up to the consumers to develop some quasi framework that minimizes the boilerplate for handling this for each and every registered command. I'm already looking into the code; it's just not easy to navigate around with something you have only been actively using for like 2 months... |
see #1208 This is anything but exhaustive, it just covers my most obvious use case. |
We know about it already, unfortunately the issue we are facing might be also caused by dependencies, our code is fine when web socket is not connected yet.. Regarding to your suggestion, we already considered about it, but it is wrong to log everything even those promise rejections already handled/caught by users and not respecting users configured Logger. DiscordPHP/src/Discord/Discord.php Lines 793 to 799 in ac16fdf
notice why it is logged only in websocket events because it only gets silence after websocket connected... Due to the nature of promise chain, where this library being in middle position, the ideal solution has to be implemented on user's end. |
No description provided.