Skip to content

Commit 9e8f672

Browse files
committed
some improvements
1 parent 3505eb8 commit 9e8f672

4 files changed

Lines changed: 88 additions & 7 deletions

File tree

app/lib/methods/actions.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,13 @@ describe('actions', () => {
102102
expect.objectContaining({
103103
method: 'POST',
104104
headers: expect.objectContaining({
105-
'X-Auth-Token': 'auth-token',
106-
'X-User-Id': 'user-id'
105+
'Content-Type': 'application/json'
107106
})
108107
})
109108
);
109+
const headersArg = mockedFetch.mock.calls[0][1]?.headers as Record<string, string>;
110+
expect(headersArg['X-Auth-Token']).toBeUndefined();
111+
expect(headersArg['X-User-Id']).toBeUndefined();
110112
});
111113

112114
it('handles modal.update response', async () => {

app/lib/methods/actions.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ export async function triggerAction({
108108
const payload = rest.payload ?? rest.value;
109109

110110
try {
111-
const { userId, authToken } = sdk.currentLogin;
112111
const host = sdk.server;
113112
if (!host) {
114113
throw new Error('SDK not initialized');
@@ -130,12 +129,12 @@ export async function triggerAction({
130129
});
131130

132131
// we need to use fetch because this.sdk.post add /v1 to url
132+
// fetch() already attaches the session headers ambiently for same-origin requests
133+
// (host === sdk.server), so we don't rebuild them here.
133134
const result = await fetch(`${host}/api/apps/ui.interaction/${appId}/`, {
134135
method: 'POST',
135136
headers: {
136-
'Content-Type': 'application/json',
137-
'X-Auth-Token': authToken,
138-
'X-User-Id': userId
137+
'Content-Type': 'application/json'
139138
},
140139
body: JSON.stringify(interaction)
141140
});

app/lib/services/sdk.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,27 @@ describe('Sdk.login', () => {
200200
expect(result.authToken).toBe('tok-abc');
201201
});
202202

203+
it('rejects instead of hanging forever if loginWithToken() never resolves (e.g. socket torn down mid-login)', async () => {
204+
jest.useFakeTimers();
205+
const loginWithToken = jest.fn(() => new Promise(() => {})); // never resolves
206+
const fake = buildFakeSdkWithLogin(
207+
{ status: 'success', data: { authToken: 'tok-abc', userId: 'uid-1', me: { username: 'john' } } },
208+
loginWithToken
209+
);
210+
setInternalSdk(fake);
211+
212+
const loginPromise = sdk.login({ user: 'john', password: 'secret' });
213+
// Attach the rejection expectation before advancing the fake timer: login() awaits its own
214+
// REST call before reaching the timeout race, and advanceTimersByTimeAsync flushes pending
215+
// microtasks as it advances — it can settle loginPromise before the next line would
216+
// otherwise attach a handler, which Node reports as an unhandled rejection.
217+
// eslint-disable-next-line jest/valid-expect
218+
const assertion = expect(loginPromise).rejects.toThrow('timed out');
219+
await jest.advanceTimersByTimeAsync(20000);
220+
await assertion;
221+
jest.useRealTimers();
222+
});
223+
203224
it('rejects when the REST response has status: error', async () => {
204225
const fake = buildFakeSdkWithLogin({ status: 'error' });
205226
setInternalSdk(fake);
@@ -323,6 +344,54 @@ describe('Sdk.login', () => {
323344
expect(sdk.getHeaders()['X-User-Id']).toBe('uid-2');
324345
});
325346

347+
it('rejects a login() superseded by a newer login() to the SAME server, without clobbering its headers', async () => {
348+
// Double-tapping login, or a flapping connection re-dispatching loginRequest, can fire two
349+
// concurrent login() calls at the same server. The generation guard must let only the
350+
// newer one win, regardless of which loginWithToken() call happens to resolve last.
351+
const resolvers: Array<() => void> = [];
352+
const loginWithToken = jest.fn(() => new Promise<void>(resolve => resolvers.push(resolve)));
353+
const fake = buildFakeSdkWithLogin(
354+
{ status: 'success', data: { authToken: 'tok-first', userId: 'uid-first', me: { username: 'user1' } } },
355+
loginWithToken
356+
);
357+
setInternalSdk(fake);
358+
359+
const firstLoginPromise = sdk.login({ user: 'user1', password: 'pass1' });
360+
361+
// Let the first login clear its own (pre-loginWithToken) generation check and reach
362+
// loginWithToken() before the second login starts — otherwise the second login's
363+
// generation bump would make the first fail its *early* check instead of exercising
364+
// the race this test is actually after (both suspended inside loginWithToken()).
365+
for (let i = 0; i < 10 && resolvers.length < 1; i++) {
366+
// eslint-disable-next-line no-await-in-loop
367+
await Promise.resolve();
368+
}
369+
expect(resolvers).toHaveLength(1);
370+
371+
fake.__post.mockResolvedValueOnce({
372+
status: 'success',
373+
data: { authToken: 'tok-second', userId: 'uid-second', me: { username: 'user1' } }
374+
});
375+
const secondLoginPromise = sdk.login({ user: 'user1', password: 'pass1' });
376+
377+
for (let i = 0; i < 10 && resolvers.length < 2; i++) {
378+
// eslint-disable-next-line no-await-in-loop
379+
await Promise.resolve();
380+
}
381+
expect(resolvers).toHaveLength(2);
382+
383+
// The second (newer) login resolves first and wins.
384+
resolvers[1]();
385+
await secondLoginPromise;
386+
expect(sdk.getHeaders()['X-Auth-Token']).toBe('tok-second');
387+
388+
// The first (now-superseded) login resumes — it must reject rather than clobber it.
389+
resolvers[0]();
390+
await expect(firstLoginPromise).rejects.toThrow('Superseded by a newer login attempt');
391+
expect(sdk.getHeaders()['X-Auth-Token']).toBe('tok-second');
392+
expect(sdk.getHeaders()['X-User-Id']).toBe('uid-second');
393+
});
394+
326395
it('switches to the deep link server credentials when already logged in on another server', async () => {
327396
// Already authenticated on server1.
328397
const server1Sdk = buildFakeSdkWithLogin({
@@ -599,6 +668,17 @@ describe('Sdk.methodCall', () => {
599668
expect(result).toEqual({ result: 'ok' });
600669
});
601670

671+
it('rejects instead of hanging forever if callAsyncWithOptions never resolves (e.g. socket torn down mid-call)', async () => {
672+
jest.useFakeTimers();
673+
const callAsync = jest.fn(() => new Promise(() => {})); // never resolves, as if the socket died mid-call
674+
setInternalSdk(buildFakeSdkWithMethod(callAsync));
675+
676+
const callPromise = (sdk as any).methodCall('myMethod');
677+
jest.advanceTimersByTime(20000);
678+
await expect(callPromise).rejects.toThrow('timed out');
679+
jest.useRealTimers();
680+
});
681+
602682
it('appends the resolved TOTP code to params when a retry is in flight', async () => {
603683
const callAsync = jest
604684
.fn()

app/views/ThreadMessagesView/index.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ class ThreadMessagesView extends Component<IThreadMessagesViewProps, IThreadMess
352352
offset: offset + API_FETCH_COUNT
353353
});
354354
} else {
355-
this.setState({ loading: false });
355+
this.setState({ loading: false, end: true });
356356
}
357357
} catch (e) {
358358
log(e);

0 commit comments

Comments
 (0)