Skip to content

Commit 279cb6f

Browse files
gregfromstlgregfromstl
committed
[SDK] Feature: Propagate error message from failed sign in (#5621)
<!-- start pr-codex --> ## PR-Codex overview This PR focuses on enhancing the user interface for error handling during the sign-in process in the `SignatureScreen` component of the `thirdweb` wallet. It propagates error messages to the UI, improving user feedback when sign-in fails. ### Detailed summary - Updated `walletId` parameter to accept `Wallet` or `string` in `is-ecosystem-wallet.ts`. - Changed state management in `SignatureScreen.tsx` to include an `error` state. - Modified sign-in logic to set error messages on failure. - Enhanced UI to display error messages and loading state. - Added `data-testid` attributes for testing. - Updated tests in `SignatureScreen.test.tsx` to cover new error handling scenarios and UI changes. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` <!-- end pr-codex -->
1 parent 91f3b4b commit 279cb6f

File tree

4 files changed

+306
-6
lines changed

4 files changed

+306
-6
lines changed

‎.changeset/moody-turtles-count.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"thirdweb": patch
3+
---
4+
5+
Feature: Propagate failed sign in error message to the UI

‎packages/thirdweb/src/react/web/ui/ConnectWallet/screens/SignatureScreen.test.tsx

Lines changed: 287 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,287 @@
1+
import { userEvent } from "@testing-library/user-event";
2+
import { beforeEach, describe, expect, it, vi } from "vitest";
3+
import { TEST_ACCOUNT_A } from "~test/test-wallets.js";
4+
import { render, waitFor } from "../../../../../../test/src/react-render.js";
5+
import { TEST_CLIENT } from "../../../../../../test/src/test-clients.js";
6+
import { createWallet } from "../../../../../wallets/create-wallet.js";
7+
import { useActiveWallet } from "../../../../core/hooks/wallets/useActiveWallet.js";
8+
import type { ConnectLocale } from "../locale/types.js";
9+
import { SignatureScreen } from "./SignatureScreen.js";
10+
11+
const mockAuth = vi.hoisted(() => ({
12+
doLogin: vi.fn().mockResolvedValue(undefined),
13+
doLogout: vi.fn().mockResolvedValue(undefined),
14+
getLoginPayload: vi.fn().mockResolvedValue(undefined),
15+
isLoggedIn: vi.fn().mockResolvedValue(true),
16+
}));
17+
18+
vi.mock("../../../../core/hooks/auth/useSiweAuth", () => ({
19+
useSiweAuth: () => mockAuth,
20+
}));
21+
22+
vi.mock("../../../../core/hooks/wallets/useActiveWallet", () => ({
23+
useActiveWallet: vi.fn().mockReturnValue(createWallet("io.metamask")),
24+
}));
25+
26+
vi.mock("../../../../core/hooks/wallets/useActiveAccount", () => ({
27+
useActiveAccount: () => vi.fn().mockReturnValue(TEST_ACCOUNT_A),
28+
}));
29+
30+
vi.mock("../../../../core/hooks/wallets/useAdminWallet", () => ({
31+
useAdminWallet: () => vi.fn().mockReturnValue(null),
32+
}));
33+
34+
const mockConnectLocale = {
35+
signatureScreen: {
36+
title: "Sign In",
37+
instructionScreen: {
38+
title: "Sign Message",
39+
instruction: "Please sign the message",
40+
signInButton: "Sign In",
41+
disconnectWallet: "Disconnect",
42+
},
43+
signingScreen: {
44+
title: "Signing",
45+
inProgress: "Signing in progress...",
46+
failedToSignIn: "Failed to sign in",
47+
prompt: "Please check your wallet",
48+
tryAgain: "Try Again",
49+
},
50+
},
51+
agreement: {
52+
prefix: "By connecting, you agree to our",
53+
termsOfService: "Terms of Service",
54+
and: "and",
55+
privacyPolicy: "Privacy Policy",
56+
},
57+
} as unknown as ConnectLocale;
58+
59+
describe("SignatureScreen", () => {
60+
beforeEach(() => {
61+
vi.clearAllMocks();
62+
mockAuth.doLogin.mockResolvedValue(undefined);
63+
});
64+
65+
it("renders initial state correctly", () => {
66+
const { getByTestId } = render(
67+
<SignatureScreen
68+
onDone={() => {}}
69+
modalSize="wide"
70+
connectLocale={mockConnectLocale}
71+
client={TEST_CLIENT}
72+
auth={mockAuth}
73+
/>,
74+
{ setConnectedWallet: true },
75+
);
76+
77+
expect(getByTestId("sign-in-button")).toBeInTheDocument();
78+
expect(getByTestId("disconnect-button")).toBeInTheDocument();
79+
});
80+
81+
it("handles signing flow", async () => {
82+
const onDoneMock = vi.fn();
83+
const { getByRole, getByText } = render(
84+
<SignatureScreen
85+
onDone={onDoneMock}
86+
modalSize="wide"
87+
connectLocale={mockConnectLocale}
88+
client={TEST_CLIENT}
89+
auth={mockAuth}
90+
/>,
91+
{ setConnectedWallet: true },
92+
);
93+
94+
const signInButton = getByRole("button", { name: "Sign In" });
95+
await userEvent.click(signInButton);
96+
97+
// Should show signing in progress
98+
await waitFor(() => {
99+
expect(getByText("Signing in progress...")).toBeInTheDocument();
100+
});
101+
});
102+
103+
it("shows loading state when wallet is undefined", async () => {
104+
vi.mocked(useActiveWallet).mockReturnValueOnce(undefined);
105+
106+
const { queryByTestId } = render(
107+
<SignatureScreen
108+
onDone={() => {}}
109+
modalSize="wide"
110+
connectLocale={mockConnectLocale}
111+
client={TEST_CLIENT}
112+
auth={mockAuth}
113+
/>,
114+
{ setConnectedWallet: true },
115+
);
116+
117+
expect(queryByTestId("sign-in-button")).not.toBeInTheDocument();
118+
});
119+
120+
it("handles error state", async () => {
121+
mockAuth.doLogin.mockRejectedValueOnce(new Error("Signing failed"));
122+
const { getByTestId, getByRole, getByText } = render(
123+
<SignatureScreen
124+
onDone={() => {}}
125+
modalSize="wide"
126+
connectLocale={mockConnectLocale}
127+
client={TEST_CLIENT}
128+
auth={mockAuth}
129+
/>,
130+
{ setConnectedWallet: true },
131+
);
132+
133+
const signInButton = await waitFor(() => {
134+
return getByTestId("sign-in-button");
135+
});
136+
await userEvent.click(signInButton);
137+
138+
// Should show error state
139+
await waitFor(
140+
() => {
141+
expect(getByText("Signing failed")).toBeInTheDocument();
142+
expect(getByRole("button", { name: "Try Again" })).toBeInTheDocument();
143+
},
144+
{
145+
timeout: 2000,
146+
},
147+
);
148+
});
149+
150+
describe("HeadlessSignIn", () => {
151+
const mockWallet = createWallet("inApp");
152+
beforeEach(() => {
153+
vi.mocked(useActiveWallet).mockReturnValue(mockWallet);
154+
});
155+
156+
it("automatically triggers sign in on mount", async () => {
157+
render(
158+
<SignatureScreen
159+
onDone={() => {}}
160+
modalSize="wide"
161+
connectLocale={mockConnectLocale}
162+
client={TEST_CLIENT}
163+
auth={mockAuth}
164+
/>,
165+
{ setConnectedWallet: true },
166+
);
167+
168+
await waitFor(() => {
169+
expect(mockAuth.doLogin).toHaveBeenCalledTimes(1);
170+
});
171+
});
172+
173+
it("shows signing message during signing state", async () => {
174+
const { getByText } = render(
175+
<SignatureScreen
176+
onDone={() => {}}
177+
modalSize="wide"
178+
connectLocale={mockConnectLocale}
179+
client={TEST_CLIENT}
180+
auth={mockAuth}
181+
/>,
182+
{ setConnectedWallet: true },
183+
);
184+
185+
await waitFor(() => {
186+
expect(getByText("Signing")).toBeInTheDocument();
187+
});
188+
});
189+
190+
it("shows error and retry button when signing fails", async () => {
191+
mockAuth.doLogin.mockRejectedValueOnce(
192+
new Error("Headless signing failed"),
193+
);
194+
195+
const { getByText, getByRole } = render(
196+
<SignatureScreen
197+
onDone={() => {}}
198+
modalSize="wide"
199+
connectLocale={mockConnectLocale}
200+
client={TEST_CLIENT}
201+
auth={mockAuth}
202+
/>,
203+
{ setConnectedWallet: true },
204+
);
205+
206+
await waitFor(
207+
() => {
208+
expect(getByText("Headless signing failed")).toBeInTheDocument();
209+
expect(
210+
getByRole("button", { name: "Try Again" }),
211+
).toBeInTheDocument();
212+
},
213+
{ timeout: 2000 },
214+
);
215+
});
216+
217+
it("allows retry after failure", async () => {
218+
mockAuth.doLogin
219+
.mockRejectedValueOnce(new Error("Failed first time"))
220+
.mockResolvedValueOnce(undefined);
221+
222+
const { getByRole, getByText } = render(
223+
<SignatureScreen
224+
onDone={() => {}}
225+
modalSize="wide"
226+
connectLocale={mockConnectLocale}
227+
client={TEST_CLIENT}
228+
auth={mockAuth}
229+
/>,
230+
{ setConnectedWallet: true },
231+
);
232+
233+
// Wait for initial failure
234+
await waitFor(
235+
() => {
236+
expect(getByText("Failed first time")).toBeInTheDocument();
237+
},
238+
{ timeout: 2000 },
239+
);
240+
241+
// Click retry
242+
const retryButton = getByRole("button", { name: "Try Again" });
243+
await userEvent.click(retryButton);
244+
245+
// Should show loading again
246+
await waitFor(() => {
247+
expect(getByText("Signing")).toBeInTheDocument();
248+
});
249+
250+
// Should have called login twice
251+
expect(mockAuth.doLogin).toHaveBeenCalledTimes(2);
252+
});
253+
254+
it("allows disconnecting wallet after failure", async () => {
255+
const mockDisconnect = vi.fn().mockResolvedValue(undefined);
256+
mockAuth.doLogin.mockRejectedValueOnce(new Error("Failed"));
257+
vi.mocked(useActiveWallet).mockReturnValueOnce({
258+
...createWallet("io.metamask"),
259+
disconnect: mockDisconnect,
260+
});
261+
262+
const { getByTestId } = render(
263+
<SignatureScreen
264+
onDone={() => {}}
265+
modalSize="wide"
266+
connectLocale={mockConnectLocale}
267+
client={TEST_CLIENT}
268+
auth={mockAuth}
269+
/>,
270+
{ setConnectedWallet: true },
271+
);
272+
273+
// Wait for failure and click disconnect
274+
await waitFor(
275+
() => {
276+
return getByTestId("disconnect-button");
277+
},
278+
{ timeout: 2000 },
279+
).then((button) => userEvent.click(button));
280+
281+
// Should have attempted to disconnect
282+
await waitFor(() => {
283+
expect(mockDisconnect).toHaveBeenCalled();
284+
});
285+
});
286+
});
287+
});

‎packages/thirdweb/src/react/web/ui/ConnectWallet/screens/SignatureScreen.tsx

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,24 +51,26 @@ export const SignatureScreen: React.FC<{
5151
const adminWallet = useAdminWallet();
5252
const activeAccount = useActiveAccount();
5353
const siweAuth = useSiweAuth(wallet, activeAccount, props.auth);
54-
const [status, setStatus] = useState<Status>("idle");
54+
const [error, setError] = useState<string | undefined>(undefined);
55+
const [status, setStatus] = useState<Status>(error ? "failed" : "idle");
5556
const { disconnect } = useDisconnect();
5657
const locale = connectLocale.signatureScreen;
5758

5859
const signIn = useCallback(async () => {
5960
try {
61+
setError(undefined);
6062
setStatus("signing");
6163
await siweAuth.doLogin();
6264
onDone?.();
6365
} catch (err) {
6466
await wait(1000);
67+
setError((err as Error).message);
6568
setStatus("failed");
66-
console.error("failed to log in", err);
6769
}
6870
}, [onDone, siweAuth]);
6971

7072
if (!wallet) {
71-
return <LoadingScreen />;
73+
return <LoadingScreen data-testid="loading-screen" />;
7274
}
7375

7476
if (
@@ -78,6 +80,7 @@ export const SignatureScreen: React.FC<{
7880
) {
7981
return (
8082
<HeadlessSignIn
83+
error={error}
8184
signIn={signIn}
8285
status={status}
8386
connectLocale={connectLocale}
@@ -126,6 +129,7 @@ export const SignatureScreen: React.FC<{
126129
<Button
127130
fullWidth
128131
variant="accent"
132+
data-testid="sign-in-button"
129133
onClick={signIn}
130134
style={{
131135
alignItems: "center",
@@ -138,6 +142,7 @@ export const SignatureScreen: React.FC<{
138142
<Button
139143
fullWidth
140144
variant="secondary"
145+
data-testid="disconnect-button"
141146
onClick={() => {
142147
disconnect(wallet);
143148
}}
@@ -162,7 +167,7 @@ export const SignatureScreen: React.FC<{
162167
<Container flex="column" gap="md" animate="fadein" key={status}>
163168
<Text size="lg" center color="primaryText">
164169
{status === "failed"
165-
? locale.signingScreen.failedToSignIn
170+
? error || locale.signingScreen.failedToSignIn
166171
: locale.signingScreen.inProgress}
167172
</Text>
168173

@@ -224,12 +229,14 @@ export const SignatureScreen: React.FC<{
224229

225230
function HeadlessSignIn({
226231
signIn,
232+
error,
227233
status,
228234
connectLocale,
229235
wallet,
230236
}: {
231237
signIn: () => void;
232238
status: Status;
239+
error: string | undefined;
233240
connectLocale: ConnectLocale;
234241
wallet: Wallet;
235242
}) {
@@ -262,7 +269,7 @@ function HeadlessSignIn({
262269
<Container>
263270
<Spacer y="lg" />
264271
<Text size="lg" center color="danger">
265-
{locale.signingScreen.failedToSignIn}
272+
{error || locale.signingScreen.failedToSignIn}
266273
</Text>
267274

268275
<Spacer y="lg" />
@@ -288,6 +295,7 @@ function HeadlessSignIn({
288295
onClick={() => {
289296
disconnect(wallet);
290297
}}
298+
data-testid="disconnect-button"
291299
style={{
292300
alignItems: "center",
293301
padding: spacing.md,

‎packages/thirdweb/src/wallets/ecosystem/is-ecosystem-wallet.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export function isEcosystemWallet(wallet: string): wallet is EcosystemWalletId;
1010
/**
1111
* Checks if the given wallet is an ecosystem wallet.
1212
*
13-
* @param {string} walletId - The wallet ID to check.
13+
* @param {Wallet | string} wallet - The wallet or wallet ID to check.
1414
* @returns {boolean} True if the wallet is an ecosystem wallet, false otherwise.
1515
* @internal
1616
*/

0 commit comments

Comments
 (0)