toki/mattermost-mobile
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[MM-62777] Fix errors being nested. Make MFA code more resilient to this situation (#8531)

Browse source 
* Fix errors being nested. Make code more resilient to this situation

* address review comments

* code review comment
This commit is contained in:
Guillermo Vayá 2025-01-31 07:31:15 +01:00 committed by GitHub
parent 1cc6e7c3de
commit 8782f4e3a4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 76 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
app/client/rest/tracking.test.ts
View file

@ -9,9 +9,9 @@ import {Events} from '@constants';
import test_helper from '@test/test_helper';
import * as ClientConstants from './constants';
import ClientError from './error';
import ClientTracking, {testExports} from './tracking';
import type ClientError from './error';
import type {APIClientInterface, ClientResponseMetrics} from '@mattermost/react-native-network-client';
type ParallelGroup = typeof testExports.ParallelGroup;
@ -244,13 +244,14 @@ describe('ClientTracking', () => {
try {
await client.doFetchWithTracking('https://example.com/api', options);
fail('Expected error to be thrown');
} catch (error: unknown) {
expect(error).toBeInstanceOf(ClientError);
const clientError = error as ClientError;
expect((clientError.details as {message: string}).message).toBe('Custom error message');
expect((clientError.details as {server_error_id: string}).server_error_id).toBe('error_id_123');
expect((clientError.details as {status_code: string}).status_code).toBe(400);
expect((clientError as {message: string}).message).toBe('Custom error message');
expect((clientError as {server_error_id: string}).server_error_id).toBe('error_id_123');
expect((clientError as {status_code: number}).status_code).toBe(400);
}
});
@ -272,8 +273,8 @@ describe('ClientTracking', () => {
} catch (error: unknown) {
const clientError = error as ClientError;
expect((clientError.details as {message: string}).message).toBe('Response with status code 500');
expect((clientError.details as {status_code: string}).status_code).toBe(500);
expect((clientError as {message: string}).message).toBe('Response with status code 500');
expect((clientError as {status_code: number}).status_code).toBe(500);
}
});

65
app/client/rest/tracking.ts
View file

@ -16,7 +16,7 @@ import {semverFromServerVersion} from '@utils/server';
import * as ClientConstants from './constants';
import ClientError from './error';
import type {APIClientInterface, ClientHeaders, ClientResponseMetrics, RequestOptions} from '@mattermost/react-native-network-client';
import type {APIClientInterface, ClientHeaders, ClientResponse, ClientResponseMetrics, RequestOptions} from '@mattermost/react-native-network-client';
type UrlData = {
count: number;
@ -372,38 +372,9 @@ export default class ClientTracking {
this.incrementRequestCount(groupLabel);
}
let response: ClientResponse;
try {
const response = await request!(url, this.buildRequestOptions(options));
const headers: ClientHeaders = response.headers || {};
if (groupLabel && CollectNetworkMetrics) {
this.trackRequest(groupLabel, url, response.metrics);
}
const serverVersion = semverFromServerVersion(
headers[ClientConstants.HEADER_X_VERSION_ID] || headers[ClientConstants.HEADER_X_VERSION_ID.toLowerCase()],
);
const hasCacheControl = Boolean(
headers[ClientConstants.HEADER_CACHE_CONTROL] || headers[ClientConstants.HEADER_CACHE_CONTROL.toLowerCase()],
);
if (serverVersion && !hasCacheControl && this.serverVersion !== serverVersion) {
this.serverVersion = serverVersion;
DeviceEventEmitter.emit(Events.SERVER_VERSION_CHANGED, {serverUrl: this.apiClient.baseUrl, serverVersion});
}
const bearerToken = headers[ClientConstants.HEADER_TOKEN] || headers[ClientConstants.HEADER_TOKEN.toLowerCase()];
if (bearerToken) {
this.setBearerToken(bearerToken);
}
if (response.ok) {
return returnDataOnly ? (response.data || {}) : response;
}
throw new ClientError(this.apiClient.baseUrl, {
message: response.data?.message as string || `Response with status code ${response.code}`,
server_error_id: response.data?.id as string,
status_code: response.code,
url,
});
response = await request!(url, this.buildRequestOptions(options));
} catch (error) {
throw new ClientError(this.apiClient.baseUrl, {
message: 'Received invalid response from the server.',
@ -419,5 +390,35 @@ export default class ClientTracking {
this.decrementRequestCount(groupLabel);
}
}
const headers: ClientHeaders = response.headers || {};
if (groupLabel && CollectNetworkMetrics) {
this.trackRequest(groupLabel, url, response.metrics);
}
const serverVersion = semverFromServerVersion(
headers[ClientConstants.HEADER_X_VERSION_ID] || headers[ClientConstants.HEADER_X_VERSION_ID.toLowerCase()],
);
const hasCacheControl = Boolean(
headers[ClientConstants.HEADER_CACHE_CONTROL] || headers[ClientConstants.HEADER_CACHE_CONTROL.toLowerCase()],
);
if (serverVersion && !hasCacheControl && this.serverVersion !== serverVersion) {
this.serverVersion = serverVersion;
DeviceEventEmitter.emit(Events.SERVER_VERSION_CHANGED, {serverUrl: this.apiClient.baseUrl, serverVersion});
}
const bearerToken = headers[ClientConstants.HEADER_TOKEN] || headers[ClientConstants.HEADER_TOKEN.toLowerCase()];
if (bearerToken) {
this.setBearerToken(bearerToken);
}
if (response.ok) {
return returnDataOnly ? (response.data || {}) : response;
}
throw new ClientError(this.apiClient.baseUrl, {
message: response.data?.message as string || `Response with status code ${response.code}`,
server_error_id: response.data?.id as string,
status_code: response.code,
url,
});
};
}

19
app/screens/login/form.tsx
View file

@ -17,7 +17,7 @@ import {useAvoidKeyboard} from '@hooks/device';
import {t} from '@i18n';
import {goToScreen, loginAnimationOptions, resetToHome} from '@screens/navigation';
import {buttonBackgroundStyle, buttonTextStyle} from '@utils/buttonStyles';
import {getFullErrorMessage, isErrorWithMessage, isServerError} from '@utils/errors';
import {getFullErrorMessage, getServerError, isErrorWithMessage, isServerError} from '@utils/errors';
import {preventDoubleTap} from '@utils/tap';
import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme';
import {tryOpenURL} from '@utils/url';
@ -117,20 +117,23 @@ const LoginForm = ({config, extra, keyboardAwareRef, serverDisplayName, launchEr
resetToHome({extra, launchError: hasError, launchType, serverUrl});
};
const checkLoginResponse = (data: LoginActionResponse) => {
let errorId = '';
const loginError = data.error;
if (isServerError(loginError) && loginError.server_error_id) {
errorId = loginError.server_error_id;
const isMFAError = (loginError: unknown): boolean => {
const serverError = getServerError(loginError);
if (serverError) {
return MFA_EXPECTED_ERRORS.includes(serverError);
}
return false;
};
if (data.failed && MFA_EXPECTED_ERRORS.includes(errorId)) {
const checkLoginResponse = (data: LoginActionResponse) => {
const {failed, error: loginError} = data;
if (failed && isMFAError(loginError)) {
goToMfa();
setIsLoading(false);
return false;
}
if (loginError && data.failed) {
if (failed && loginError) {
setIsLoading(false);
setError(getLoginErrorMessage(loginError));
return false;

11
app/utils/errors.test.ts
View file

@ -9,6 +9,7 @@ import {
isErrorWithStatusCode,
isErrorWithUrl,
getFullErrorMessage,
getServerError,
} from './errors';
import {getIntlShape} from './general';
@ -69,4 +70,14 @@ describe('Errors', () => {
details: 'more info',
})).toBe('Unknown error; more info');
});
test('getServerError', () => {
expect(getServerError({message: 'error message'})).toBe(undefined);
expect(getServerError({details: 'more info', message: 'error message'})).toBe(undefined);
expect(getServerError({server_error_id: 'server error', message: 'error message'})).toBe('server error');
expect(getServerError({details: {server_error_id: 'deep error'}, message: 'error message'})).toBe('deep error');
expect(getServerError({details: {details: {server_error_id: 'deeper error'}}, message: 'error message'})).toBe('deeper error');
expect(getServerError({details: {details: {details: {details: {server_error_id: 'too deep error'}}}}, message: 'error message'})).toBe(undefined);
expect(getServerError({server_error_id: 'server error', details: {server_error_id: 'not this'}, message: 'error message'})).toBe('server error');
});
});

13
app/utils/errors.ts
View file

@ -87,3 +87,16 @@ export const getErrorMessage = (error: unknown, intl?: IntlShape) => {
return 'Unknown error';
};
export const getServerError = (error: unknown, depth = 0): string | undefined => {
if (isServerError(error)) {
return error.server_error_id!;
}
if (isErrorWithDetails(error)) {
if (depth > 2) {
return undefined;
}
return getServerError(error.details, depth + 1);
}
return undefined;
};