Add CloudDevice service class#1709
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the CloudDeviceAPI service to support synchronous and asynchronous payment requests, retrieving connected devices, and checking device status, complete with unit tests and mock data. Feedback on the implementation highlights a potential runtime crash and type unsafety in the async method when parsing responses, as well as unintended side effects caused by mutating the requestOptions parameter directly in getConnectedDevices.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const response = await getJsonResponse<CloudDeviceApiRequest, string>( | ||
| resource, | ||
| cloudDeviceApiRequest, | ||
| { ...requestOptions, method: "POST" } | ||
| ); | ||
| const result = new CloudDeviceApiAsyncResponse(); | ||
| if (typeof response === "string" && response.toLowerCase().trim() === "ok") { | ||
| result.Result = response.trim(); | ||
| } else { | ||
| const parsed: CloudDeviceApiAsyncResponse = typeof response === "string" ? JSON.parse(response) : response; | ||
| result.SaleToPOIRequest = parsed.SaleToPOIRequest; | ||
| } | ||
| return result; |
There was a problem hiding this comment.
The current implementation of the async method is prone to runtime errors and contains type unsafety:
- Potential Crash on Empty/Invalid Response: If the API returns an empty response (which
getJsonResponseresolves to"") or an invalid JSON string, callingJSON.parse(response)will throw an unhandledSyntaxErrorand crash the execution. - Redundant Parsing:
getJsonResponsealready attempts to parse the response string into a JSON object. If successful, it returns the parsed object. Thus, callingJSON.parseagain on an already parsed object (or on a string that failed parsing) is redundant and error-prone. - Type Lie: Typing
getJsonResponsewithstringas the response type (R) forces TypeScript to treat the parsed object as a string, leading to manual type casting and potential runtime issues.
We can resolve all these issues cleanly by typing getJsonResponse with CloudDeviceApiAsyncResponse and checking the type of the returned response.
const response = await getJsonResponse<CloudDeviceApiRequest, CloudDeviceApiAsyncResponse>(
resource,
cloudDeviceApiRequest,
{ ...requestOptions, method: "POST" }
);
const result = new CloudDeviceApiAsyncResponse();
if (typeof response === "string") {
if (response.toLowerCase().trim() === "ok") {
result.Result = response.trim();
}
} else if (response && typeof response === "object") {
result.SaleToPOIRequest = response.SaleToPOIRequest;
}
return result;| const hasDefinedQueryParams = store; | ||
| if (hasDefinedQueryParams) { | ||
| if (!requestOptions) requestOptions = {}; | ||
| if (!requestOptions.params) requestOptions.params = {}; | ||
| if (store) (requestOptions.params as Record<string, string>)["store"] = store; | ||
| } | ||
| const response = await getJsonResponse<string, ConnectedDevicesResponse>( | ||
| resource, | ||
| "", | ||
| { ...requestOptions, method: "GET" } | ||
| ); |
There was a problem hiding this comment.
Mutating the requestOptions argument passed by the caller is a side effect that can lead to unexpected behavior if the caller reuses the same options object for subsequent requests.
Instead of mutating requestOptions directly, we should create a shallow copy of the options and append the query parameters safely.
const options: IRequest.Options = { ...requestOptions, method: "GET" };
if (store) {
options.params = { ...options.params, store };
}
const response = await getJsonResponse<string, ConnectedDevicesResponse>(
resource,
"",
options
);
Add
CloudDeviceApiservice class to support the Cloud Device API endpoints:/merchants/{merchantAccount}/devices/{deviceId}/sync/merchants/{merchantAccount}/devices/{deviceId}/async/merchants/{merchantAccount}/connectedDevices/merchants/{merchantAccount}/devices/{deviceId}/statusAdded supporting models (which are not part of the OpenAPI spec), unit tests and mock data.