Skip to content
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

Properly reject the promise in client initialisation #167

Merged

Conversation

Arakmar
Copy link
Contributor

@Arakmar Arakmar commented Aug 14, 2022

Changes

In case of an authentication error or an API outage during client initialisation, it's not possible to catch the error.

Status

  • These changes have been tested and documented properly.
  • This pull request introduces some breaking changes.

@adb-sh
Copy link
Contributor

adb-sh commented Oct 12, 2022

looks good. this should fix #14 (reply in thread)

Copy link
Contributor

@adb-sh adb-sh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good in general. just added some notes for the ClientOptions type

src/Client.ts Outdated
@@ -148,7 +148,7 @@ export class Client {
* @example const client = await Client.create({ token: "token" });
*/
public static create(options: Omit<ClientOptions, 'onReady'>): Promise<Client> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably also omit onFail like: Omit<ClientOptions, 'onReady' | 'onFail'>

@Arakmar Arakmar force-pushed the bugfix/client_promise_reject branch from ccdb2cc to 96b01f6 Compare November 1, 2022 11:08
@scientific-dev scientific-dev merged commit e44e8f4 into spotify-api:master Nov 11, 2022
@scientific-dev
Copy link
Member

The changes have been update in v9.2.5. If you encounter any bugs, open a new issue to resolve it.

Thanks for your support 👍 !

@adb-sh
Copy link
Contributor

adb-sh commented Nov 11, 2022 via email

@scientific-dev
Copy link
Member

scientific-dev commented Nov 11, 2022

Just now, noticed it.
Still, thanks for your response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants