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

[TS migration] Migrate 'HttpUtils.js' lib to TypeScript #28691

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/Alert/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import _ from 'underscore';
*
* @param {String} title The title of the alert
* @param {String} description The description of the alert
* @param {Object[]} options An array of objects with `style` and `onPress` properties
* @param {Object[]} [options] An array of objects with `style` and `onPress` properties
*/
export default (title, description, options) => {
const result = _.filter(window.confirm([title, description], Boolean)).join('\n');
Expand Down
68 changes: 32 additions & 36 deletions src/libs/HttpUtils.js → src/libs/HttpUtils.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import Onyx from 'react-native-onyx';
import _ from 'underscore';
import {ValueOf} from 'type-fest';
import alert from '@components/Alert';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type Response from '@src/types/onyx/Response';
import * as ApiUtils from './ApiUtils';
import HttpsError from './Errors/HttpsError';

let shouldFailAllRequests = false;
let shouldForceOffline = false;

Onyx.connect({
key: ONYXKEYS.NETWORK,
callback: (network) => {
Expand All @@ -25,14 +27,8 @@ let cancellationController = new AbortController();
/**
* Send an HTTP request, and attempt to resolve the json response.
* If there is a network error, we'll set the application offline.
*
* @param {String} url
* @param {String} [method]
* @param {Object} [body]
* @param {Boolean} [canCancel]
* @returns {Promise}
*/
function processHTTPRequest(url, method = 'get', body = null, canCancel = true) {
function processHTTPRequest(url: string, method = 'get', body: FormData | null = null, canCancel = true): Promise<Response> {
return fetch(url, {
// We hook requests to the same Controller signal, so we can cancel them all at once
signal: canCancel ? cancellationController.signal : undefined,
Expand All @@ -49,40 +45,41 @@ function processHTTPRequest(url, method = 'get', body = null, canCancel = true)

if (!response.ok) {
// Expensify site is down or there was an internal server error, or something temporary like a Bad Gateway, or unknown error occurred
const serviceInterruptedStatuses = [
const serviceInterruptedStatuses: Array<ValueOf<typeof CONST.HTTP_STATUS>> = [
CONST.HTTP_STATUS.INTERNAL_SERVER_ERROR,
CONST.HTTP_STATUS.BAD_GATEWAY,
CONST.HTTP_STATUS.GATEWAY_TIMEOUT,
CONST.HTTP_STATUS.UNKNOWN_ERROR,
];
if (_.contains(serviceInterruptedStatuses, response.status)) {
if (serviceInterruptedStatuses.includes(response.status as ValueOf<typeof CONST.HTTP_STATUS>)) {
fvlvte marked this conversation as resolved.
Show resolved Hide resolved
throw new HttpsError({
message: CONST.ERROR.EXPENSIFY_SERVICE_INTERRUPTED,
status: response.status,
status: response.status.toString(),
title: 'Issue connecting to Expensify site',
});
} else if (response.status === CONST.HTTP_STATUS.TOO_MANY_REQUESTS) {
}
if (response.status === CONST.HTTP_STATUS.TOO_MANY_REQUESTS) {
throw new HttpsError({
message: CONST.ERROR.THROTTLED,
status: response.status,
status: response.status.toString(),
title: 'API request throttled',
});
}

throw new HttpsError({
message: response.statusText,
status: response.status,
status: response.status.toString(),
});
}

return response.json();
return response.json() as Promise<Response>;
})
.then((response) => {
// Some retried requests will result in a "Unique Constraints Violation" error from the server, which just means the record already exists
if (response.jsonCode === CONST.JSON_CODE.BAD_REQUEST && response.message === CONST.ERROR_TITLE.DUPLICATE_RECORD) {
throw new HttpsError({
message: CONST.ERROR.DUPLICATE_RECORD,
status: CONST.JSON_CODE.BAD_REQUEST,
status: CONST.JSON_CODE.BAD_REQUEST.toString(),
title: CONST.ERROR_TITLE.DUPLICATE_RECORD,
});
}
Expand All @@ -91,43 +88,42 @@ function processHTTPRequest(url, method = 'get', body = null, canCancel = true)
if (response.jsonCode === CONST.JSON_CODE.EXP_ERROR && response.title === CONST.ERROR_TITLE.SOCKET && response.type === CONST.ERROR_TYPE.SOCKET) {
throw new HttpsError({
message: CONST.ERROR.EXPENSIFY_SERVICE_INTERRUPTED,
status: CONST.JSON_CODE.EXP_ERROR,
status: CONST.JSON_CODE.EXP_ERROR.toString(),
title: CONST.ERROR_TITLE.SOCKET,
});
}
if (response.jsonCode === CONST.JSON_CODE.MANY_WRITES_ERROR) {
const {phpCommandName, authWriteCommands} = response.data;
// eslint-disable-next-line max-len
const message = `The API call (${phpCommandName}) did more Auth write requests than allowed. Count ${authWriteCommands.length}, commands: ${authWriteCommands.join(
', ',
)}. Check the APIWriteCommands class in Web-Expensify`;
alert('Too many auth writes', message);
if (response.data) {
const {phpCommandName, authWriteCommands} = response.data;
// eslint-disable-next-line max-len
const message = `The API call (${phpCommandName}) did more Auth write requests than allowed. Count ${authWriteCommands.length}, commands: ${authWriteCommands.join(
', ',
)}. Check the APIWriteCommands class in Web-Expensify`;
alert('Too many auth writes', message);
}
}
return response;
return response as Promise<Response>;
});
}

/**
* Makes XHR request
* @param {String} command the name of the API command
* @param {Object} data parameters for the API command
* @param {String} type HTTP request type (get/post)
* @param {Boolean} shouldUseSecure should we use the secure server
* @returns {Promise}
* @param command the name of the API command
* @param data parameters for the API command
* @param type HTTP request type (get/post)
* @param shouldUseSecure should we use the secure server
*/
function xhr(command, data, type = CONST.NETWORK.METHOD.POST, shouldUseSecure = false) {
function xhr(command: string, data: Record<string, unknown>, type: 'get' | 'post' = CONST.NETWORK.METHOD.POST, shouldUseSecure = false): Promise<Response> {
const formData = new FormData();
_.each(data, (val, key) => {
// Do not send undefined request parameters to our API. They will be processed as strings of 'undefined'.
if (_.isUndefined(val)) {
Object.keys(data).forEach((key) => {
if (!data[key]) {
Copy link
Contributor

@iwiznia iwiznia Nov 21, 2023

Choose a reason for hiding this comment

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

This is not equivalent to the old code... this will remove any 0, false, null or "" while the old code would not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true, changed to typeof.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jjcoffee FYI since you missed this in your review and I have the feeling this would've broken a ton of things.

Copy link
Contributor

Choose a reason for hiding this comment

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

Damn, good catch! Sorry for missing that 🙇

return;
}

formData.append(key, val);
formData.append(key, data[key] as string | Blob);
});

const url = ApiUtils.getCommandURL({shouldUseSecure, command});
return processHTTPRequest(url, type, formData, data.canCancel);
return processHTTPRequest(url, type, formData, Boolean(data.canCancel));
}

function cancelPendingRequests() {
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function makeXHR(request: Request): Promise<Response | void> {
return new Promise<void>((resolve) => resolve());
}

return HttpUtils.xhr(request.command, finalParameters, request.type, request.shouldUseSecure) as Promise<Response>;
return HttpUtils.xhr(request.command, finalParameters, request.type, request.shouldUseSecure);
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/types/onyx/Request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type RequestData = {
command: string;
commandName?: string;
data?: Record<string, unknown>;
type?: string;
type?: 'get' | 'post';
fvlvte marked this conversation as resolved.
Show resolved Hide resolved
shouldUseSecure?: boolean;
successData?: OnyxUpdate[];
failureData?: OnyxUpdate[];
Expand Down
8 changes: 8 additions & 0 deletions src/types/onyx/Response.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import {OnyxUpdate} from 'react-native-onyx';

type Data = {
phpCommandName: string;
authWriteCommands: string[];
};

type Response = {
previousUpdateID?: number | string;
lastUpdateID?: number | string;
Expand All @@ -10,6 +15,9 @@ type Response = {
authToken?: string;
encryptedAuthToken?: string;
message?: string;
title?: string;
data?: Data;
type?: string;
shortLivedAuthToken?: string;
};

Expand Down
Loading