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

Update request.ts #928

Merged
merged 4 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
8 changes: 4 additions & 4 deletions utilities/project-factory/src/server/api/campaignApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ const createBatchRequest = async (request: any, batch: any[], mobileNumberRowNum
includeDeleted: true
};
logger.info("Individual search to validate the mobile no initiated");
const response = await httpRequest(config.host.healthIndividualHost + config.paths.healthIndividualSearch, searchBody, params);
const response = await httpRequest(config.host.healthIndividualHost + config.paths.healthIndividualSearch, searchBody, params, undefined, undefined, undefined, undefined, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider handling potential undefined values in the optional chaining in the httpRequest call.

- const response = await httpRequest(config.host.healthIndividualHost + config.paths.healthIndividualSearch, searchBody, params, undefined, undefined, undefined, undefined, true);
+ const response = await httpRequest(config.host.healthIndividualHost + config.paths.healthIndividualSearch, searchBody, params ?? {}, undefined, undefined, undefined, undefined, true);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const response = await httpRequest(config.host.healthIndividualHost + config.paths.healthIndividualSearch, searchBody, params, undefined, undefined, undefined, undefined, true);
const response = await httpRequest(config.host.healthIndividualHost + config.paths.healthIndividualSearch, searchBody, params ?? {}, undefined, undefined, undefined, undefined, true);


if (!response) {
throwError("COMMON", 400, "INTERNAL_SERVER_ERROR", "Error occurred during user search while validating mobile number.");
Expand Down Expand Up @@ -465,7 +465,7 @@ async function getEmployeesBasedOnUuids(dataToCreate: any[], request: any) {
};

try {
const response = await httpRequest(searchUrl, searchBody, params);
const response = await httpRequest(searchUrl, searchBody, params, undefined, undefined, undefined, undefined, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

The error handling in the httpRequest call should be improved to handle potential undefined values safely.

- const response = await httpRequest(searchUrl, searchBody, params, undefined, undefined, undefined, undefined, true);
+ const response = await httpRequest(searchUrl, searchBody, params ?? {}, undefined, undefined, undefined, undefined, true);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const response = await httpRequest(searchUrl, searchBody, params, undefined, undefined, undefined, undefined, true);
const response = await httpRequest(searchUrl, searchBody, params ?? {}, undefined, undefined, undefined, undefined, true);

if (response && response.Employees) {
employeesSearched = employeesSearched.concat(response.Employees);
} else {
Expand Down Expand Up @@ -672,7 +672,7 @@ async function handleUserProcess(request: any, createAndSearchConfig: any, param
newRequestBody.Employees = Employees
}
if (newRequestBody.Employees.length > 0) {
var responsePayload = await httpRequest(createAndSearchConfig?.createBulkDetails?.url, newRequestBody, params, "post", undefined, undefined, true);
var responsePayload = await httpRequest(createAndSearchConfig?.createBulkDetails?.url, newRequestBody, params, "post", undefined, undefined, true, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure that params are defined to avoid potential runtime errors in the httpRequest call.

- var responsePayload = await httpRequest(createAndSearchConfig?.createBulkDetails?.url, newRequestBody, params, "post", undefined, undefined, true, true);
+ var responsePayload = await httpRequest(createAndSearchConfig?.createBulkDetails?.url, newRequestBody, params ?? {}, "post", undefined, undefined, true, true);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var responsePayload = await httpRequest(createAndSearchConfig?.createBulkDetails?.url, newRequestBody, params, "post", undefined, undefined, true, true);
var responsePayload = await httpRequest(createAndSearchConfig?.createBulkDetails?.url, newRequestBody, params ?? {}, "post", undefined, undefined, true, true);
Tools
Biome

[error] 675-675: This var should be declared at the root of the enclosing function. (lint/correctness/noInnerDeclarations)

The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.

if (responsePayload?.Employees && responsePayload?.Employees?.length > 0) {
enrichDataToCreateForUser(dataToCreate, responsePayload, request);
}
Expand Down Expand Up @@ -908,7 +908,7 @@ async function confirmProjectParentCreation(request: any, projectId: any) {
async function projectCreate(projectCreateBody: any, request: any) {
logger.info("Project creation url " + config.host.projectHost + config.paths.projectCreate)
logger.debug("Project creation body " + getFormattedStringForDebug(projectCreateBody))
const projectCreateResponse = await httpRequest(config.host.projectHost + config.paths.projectCreate, projectCreateBody);
const projectCreateResponse = await httpRequest(config.host.projectHost + config.paths.projectCreate, projectCreateBody, undefined, undefined, undefined, undefined, undefined, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

The call to httpRequest should include a check for undefined values in params to prevent potential issues.

- const projectCreateResponse = await httpRequest(config.host.projectHost + config.paths.projectCreate, projectCreateBody, undefined, undefined, undefined, undefined, undefined, true);
+ const projectCreateResponse = await httpRequest(config.host.projectHost + config.paths.projectCreate, projectCreateBody, {}, undefined, undefined, undefined, undefined, true);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const projectCreateResponse = await httpRequest(config.host.projectHost + config.paths.projectCreate, projectCreateBody, undefined, undefined, undefined, undefined, undefined, true);
const projectCreateResponse = await httpRequest(config.host.projectHost + config.paths.projectCreate, projectCreateBody, {}, undefined, undefined, undefined, undefined, true);

logger.debug("Project creation response" + getFormattedStringForDebug(projectCreateResponse))
if (projectCreateResponse?.Project[0]?.id) {
logger.info("Project created successfully with name " + JSON.stringify(projectCreateResponse?.Project[0]?.name))
Expand Down
22 changes: 15 additions & 7 deletions utilities/project-factory/src/server/api/genericApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import FormData from "form-data"; // Import FormData for handling multipart/form
import { httpRequest } from "../utils/request"; // Import httpRequest function for making HTTP requests
import { getFormattedStringForDebug, logger } from "../utils/logger"; // Import logger for logging
import { correctParentValues, findMapValue, generateActivityMessage, getBoundaryRelationshipData, getDataSheetReady, getLocalizedHeaders, sortCampaignDetails, throwError } from "../utils/genericUtils"; // Import utility functions
import { validateProjectFacilityResponse, validateProjectResourceResponse, validateStaffResponse } from "../validators/genericValidator"; // Import validation functions
import { extractCodesFromBoundaryRelationshipResponse, generateFilteredBoundaryData, getConfigurableColumnHeadersBasedOnCampaignType, getFiltersFromCampaignSearchResponse, getLocalizedName } from '../utils/campaignUtils'; // Import utility functions
import { getCampaignSearchResponse, getHierarchy } from './campaignApis';
import { validateMappingId } from '../utils/campaignMappingUtils';
Expand Down Expand Up @@ -670,14 +669,17 @@ async function createStaff(resouceBody: any) {
undefined,
"post",
undefined,
undefined
undefined,
undefined,
false,
true
Comment on lines +672 to +675
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider simplifying the function signature by removing unused parameters.

- const staffResponse = await httpRequest(
-   staffCreateUrl,
-   resouceBody,
-   undefined,
-   "post",
-   undefined,
-   undefined,
-   undefined,
-   false,
-   true
- );
+ const staffResponse = await httpRequest(staffCreateUrl, resouceBody, "post", false, true);

The same refactoring can be applied to the createProjectResource and createProjectFacility functions to make them cleaner and more readable.

Also applies to: 701-704, 730-733

);
logger.info("Project Staff mapping created");
logger.debug(
"Project Staff mapping response " +
getFormattedStringForDebug(staffResponse)
);
validateStaffResponse(staffResponse);
// validateStaffResponse(staffResponse);
}

/**
Expand All @@ -696,14 +698,17 @@ async function createProjectResource(resouceBody: any) {
undefined,
"post",
undefined,
undefined
undefined,
undefined,
false,
true
);
logger.debug("Project Resource Created");
logger.debug(
"Project Resource Creation response :: " +
getFormattedStringForDebug(projectResourceResponse)
);
validateProjectResourceResponse(projectResourceResponse);
// validateProjectResourceResponse(projectResourceResponse);
}

/**
Expand All @@ -722,14 +727,17 @@ async function createProjectFacility(resouceBody: any) {
undefined,
"post",
undefined,
undefined
undefined,
undefined,
false,
true
);
logger.info("Project Facility Created");
logger.debug(
"Project Facility Creation response" +
getFormattedStringForDebug(projectFacilityResponse)
);
validateProjectFacilityResponse(projectFacilityResponse);
// validateProjectFacilityResponse(projectFacilityResponse);
}

// Helper function to create staff
Expand Down
1 change: 1 addition & 0 deletions utilities/project-factory/src/server/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ const config = {
matchFacilityData: false,
retryCount: process.env.CREATE_RESOURCE_RETRY_COUNT || "3",
notCreateUserIfAlreadyThere: process.env.NOT_CREATE_USER_IF_ALREADY_THERE || false,
maxHttpRetries: "4"
}
};
// Exporting getErrorCodes function and config object
Expand Down
170 changes: 99 additions & 71 deletions utilities/project-factory/src/server/utils/request.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Response } from "express"; // Importing necessary module Response from Express
import { getFormattedStringForDebug, logger } from "./logger"; // Importing logger from logger module
import { cacheResponse, getCachedResponse, throwErrorViaRequest } from "./genericUtils"; // Importing necessary functions from genericUtils module
import config from "../config";

var Axios = require("axios").default; // Importing axios library
var get = require("lodash/get"); // Importing get function from lodash library
Expand Down Expand Up @@ -64,88 +65,115 @@ const httpRequest = async (
_method: string = "post",
responseType: string = "",
headers: any = defaultheader,
sendStatusCode: any = false
sendStatusCode: any = false,
retry: any = false,
dontThrowError: any = false
Comment on lines +68 to +70
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure the new parameters retry and dontThrowError are strongly typed. This aligns with best practices for TypeScript and helps prevent runtime errors.

-  retry: any = false,
-  dontThrowError: any = false
+  retry: boolean = false,
+  dontThrowError: boolean = false
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sendStatusCode: any = false,
retry: any = false,
dontThrowError: any = false
sendStatusCode: any = false,
retry: boolean = false,
dontThrowError: boolean = false

): Promise<any> => {
try {
if (headers && headers.cachekey && cacheEnabled) {
const cacheData = getCachedResponse(headers.cachekey); // Get cached response data
if (cacheData) {
return cacheData; // Return cached data if available
let attempt = 0; // Track the number of attempts
const maxAttempts = parseInt(config.values.maxHttpRetries) || 4; // Maximum number of retry attempts

while (attempt < maxAttempts) {
try {
if (headers && headers.cachekey && cacheEnabled) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use optional chaining to improve code safety and readability.

-      if (headers && headers.cachekey && cacheEnabled) {
+      if (headers?.cachekey && cacheEnabled) {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (headers && headers.cachekey && cacheEnabled) {
if (headers?.cachekey && cacheEnabled) {
Tools
Biome

[error] 77-77: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.

const cacheData = getCachedResponse(headers.cachekey); // Get cached response data
if (cacheData) {
return cacheData; // Return cached data if available
}
logger.info(
"NO CACHE FOUND :: REQUEST :: " +
JSON.stringify(headers.cachekey)
);
ashish-egov marked this conversation as resolved.
Show resolved Hide resolved
}
logger.info(
"NO CACHE FOUND :: REQUEST :: " +
JSON.stringify(headers.cachekey)
"INTER-SERVICE :: REQUEST :: " +
getServiceName(_url) +
" CRITERIA :: " +
JSON.stringify(_params)
);
}
logger.info(
"INTER-SERVICE :: REQUEST :: " +
getServiceName(_url) +
" CRITERIA :: " +
JSON.stringify(_params)
);
logger.debug("INTER-SERVICE :: REQUESTBODY :: " + getFormattedStringForDebug(_requestBody))
// Make HTTP request using Axios
const response = await Axios({
method: _method,
url: _url,
data: _requestBody,
params: _params,
headers: { ...defaultheader, ...headers },
responseType,
});
logger.debug("INTER-SERVICE :: REQUESTBODY :: " + getFormattedStringForDebug(_requestBody))
// Make HTTP request using Axios
const response = await axiosInstance({
method: _method,
url: _url,
data: _requestBody,
params: _params,
headers: { ...defaultheader, ...headers },
responseType,
});

const responseStatus = parseInt(get(response, "status"), 10); // Get response status
logger.info(
"INTER-SERVICE :: SUCCESS :: " +
getServiceName(_url) +
":: CODE :: " +
responseStatus
);
logger.debug("INTER-SERVICE :: RESPONSEBODY :: " +getFormattedStringForDebug(response.data));
const responseStatus = parseInt(get(response, "status"), 10); // Get response status
logger.info(
"INTER-SERVICE :: SUCCESS :: " +
getServiceName(_url) +
":: CODE :: " +
responseStatus
);
logger.debug("INTER-SERVICE :: RESPONSEBODY :: " + getFormattedStringForDebug(response.data));

// If response status is successful, cache the response data if caching is enabled
if (responseStatus === 200 || responseStatus === 201 || responseStatus === 202) {
if (headers && headers.cachekey) {
cacheResponse(response.data, headers.cachekey)
// If response status is successful, cache the response data if caching is enabled
if (responseStatus === 200 || responseStatus === 201 || responseStatus === 202) {
if (headers && headers.cachekey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use optional chaining to simplify the condition.

-        if (headers && headers.cachekey) {
+        if (headers?.cachekey) {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (headers && headers.cachekey) {
if (headers?.cachekey) {
Tools
Biome

[error] 115-115: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.

cacheResponse(response.data, headers.cachekey);
}
ashish-egov marked this conversation as resolved.
Show resolved Hide resolved
// Return response data with status code if sendStatusCode flag is false
if (!sendStatusCode)
return response.data;
else return { ...response.data, "statusCode": responseStatus };
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove unnecessary else clause to enhance readability.

-        else return { ...response.data, "statusCode": responseStatus };
+        return !sendStatusCode ? response.data : { ...response.data, "statusCode": responseStatus };
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
else return { ...response.data, "statusCode": responseStatus };
return !sendStatusCode ? response.data : { ...response.data, "statusCode": responseStatus };
Tools
Biome

[error] 121-121: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.

}
// Return response data with status code if sendStatusCode flag is false
if (!sendStatusCode)
return response.data;
else return { ...response.data, "statusCode": responseStatus }
}
} catch (error: any) {
var errorResponse = error?.response; // Get error response
// Log error details
logger.error(
"INTER-SERVICE :: FAILURE :: " +
getServiceName(_url) +
":: CODE :: " +
errorResponse?.status +
":: ERROR :: " +
errorResponse?.data?.Errors?.[0]?.code || error +
":: DESCRIPTION :: " +
errorResponse?.data?.Errors?.[0]?.description
);
logger.error("error occured while making request to " +
getServiceName(_url) +
": error response :" +
(errorResponse ? parseInt(errorResponse?.status, 10) : error?.message))
logger.error(":: ERROR STACK :: " + error?.stack || error);
// Throw error response via request if error response contains errors
if (errorResponse?.data?.Errors?.[0]) {
errorResponse.data.Errors[0].status = errorResponse?.data?.Errors?.[0]?.status || errorResponse?.status
throwErrorViaRequest(errorResponse?.data?.Errors?.[0]);
}
else {
// Throw error message via request
throwErrorViaRequest(
"error occured while making request to " +
} catch (error: any) {
var errorResponse = error?.response; // Get error response
Copy link
Contributor

Choose a reason for hiding this comment

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

Declare 'var errorResponse' at the top of the function to follow best practices.

+  var errorResponse;
+  // rest of the function code
-  var errorResponse = error?.response; // Get error response
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var errorResponse = error?.response; // Get error response
var errorResponse;
// rest of the function code
errorResponse = error?.response; // Get error response
Tools
Biome

[error] 124-124: This var should be declared at the root of the enclosing function. (lint/correctness/noInnerDeclarations)

The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.

// Log error details
logger.error(
"INTER-SERVICE :: FAILURE :: " +
getServiceName(_url) +
": error response :" +
(errorResponse ? parseInt(errorResponse?.status, 10) : error?.message)
":: CODE :: " +
errorResponse?.status +
":: ERROR :: " +
errorResponse?.data?.Errors?.[0]?.code || error +
":: DESCRIPTION :: " +
errorResponse?.data?.Errors?.[0]?.description
);
logger.error("error occurred while making request to " +
getServiceName(_url) +
": error response :" +
(errorResponse ? parseInt(errorResponse?.status, 10) : error?.message))
logger.error(":: ERROR STACK :: " + error?.stack || error);
if (retry) {
attempt++; // Increment the attempt count
if (attempt >= maxAttempts) {
if (dontThrowError) {
return errorResponse?.data || { Errors: [{ code: error.message, description: error.stack }] };
}
else {
throwTheHttpError(errorResponse, error, _url);
}
Comment on lines +149 to +151
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove unnecessary else clause to enhance readability.

-          else {
-            throwTheHttpError(errorResponse, error, _url);
-          }
+          throwTheHttpError(errorResponse, error, _url);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
else {
throwTheHttpError(errorResponse, error, _url);
}
throwTheHttpError(errorResponse, error, _url);
Tools
Biome

[error] 149-151: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.

}
// Wait for a short period before retrying
logger.info(`Waiting for 20 seconds before retrying httprequest with url ${_url}`);
await new Promise(resolve => setTimeout(resolve, 20000));
} else if (dontThrowError) {
return errorResponse?.data || { Errors: [{ code: error.message, description: error.stack }] };
} else {
throwTheHttpError(errorResponse, error, _url);
}
}
}
};

function throwTheHttpError(errorResponse?: any, error?: any, _url?: string) {
// Throw error response via request if error response contains errors
if (errorResponse?.data?.Errors?.[0]) {
errorResponse.data.Errors[0].status = errorResponse?.data?.Errors?.[0]?.status || errorResponse?.status
throwErrorViaRequest(errorResponse?.data?.Errors?.[0]);
} else {
// Throw error message via request
throwErrorViaRequest(
"error occurred while making request to " +
getServiceName(_url) +
": error response :" +
(errorResponse ? parseInt(errorResponse?.status, 10) : error?.message)
);
}
}

export { httpRequest }; // Exporting the httpRequest function for use in other modules
Loading
Loading