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

Fix infinite spinner for login that results in failure #1305

Merged
merged 3 commits into from
Jun 16, 2023

Conversation

ayan4m1
Copy link
Contributor

@ayan4m1 ayan4m1 commented Jun 15, 2023

Animation

Added error handling to the login form, so it doesn't just "spin infinitely" when there is an error. Error is surfaced as a toast using existing code, just calling i.setState() in the catch block.

@SleeplessOne1917
Copy link
Member

Your code looks good, but I'm confused as to why the exception isn't already being captured in the API wrapper and returning a result with state = failed.

@ayan4m1
Copy link
Contributor Author

ayan4m1 commented Jun 15, 2023

Does this help at all?

Screenshot 2023-06-15 094951

@SleeplessOne1917
Copy link
Member

SleeplessOne1917 commented Jun 15, 2023

Here's some relevant code from HttpService.ts:

class WrappedLemmyHttpClient {
  #client: LemmyHttp;

  constructor(client: LemmyHttp) {
    this.#client = client;

    for (const key of Object.getOwnPropertyNames(
      Object.getPrototypeOf(this.#client)
    )) {
      if (key !== "constructor") {
        WrappedLemmyHttpClient.prototype[key] = async (...args) => {
          try {
            const res = await this.#client[key](...args);

            return {
              data: res,
              state: "success",
            };
          } catch (error) {
            console.error(`API error: ${error}`);
            toast(i18n.t(error), "danger");
            return {
              state: "failed",
              msg: error,
            };
          }
        };
      }
    }
  }
}

Even though console.error is called in the catch block and an error toast is displayed, the client should still return a a RequestState object.

@ayan4m1 ayan4m1 force-pushed the feature/login-error branch from 5915077 to 39b9dc8 Compare June 16, 2023 14:03
@ayan4m1 ayan4m1 changed the title Wrap login call in try/catch for error handling Fix infinite spinner for login that results in failure Jun 16, 2023
@ayan4m1
Copy link
Contributor Author

ayan4m1 commented Jun 16, 2023

@SleeplessOne1917 I have updated the code to address your points, take a look when you can?

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

Ahh thanks for this one, sry I missed it!

@ayan4m1 ayan4m1 force-pushed the feature/login-error branch from a180f96 to ccf9554 Compare June 16, 2023 17:12
@ayan4m1 ayan4m1 force-pushed the feature/login-error branch from ccf9554 to 93e3598 Compare June 16, 2023 22:18
@SleeplessOne1917 SleeplessOne1917 merged commit 1242d8d into LemmyNet:main Jun 16, 2023
@ayan4m1 ayan4m1 deleted the feature/login-error branch June 16, 2023 22:34
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