From d8b4370412c6ef4f8fddd4e4bd5ce8e832278ef7 Mon Sep 17 00:00:00 2001 From: tjcouch-sil Date: Wed, 12 Jun 2024 11:44:59 +0900 Subject: [PATCH] Fixed dotnet data provider not waiting for settings service so crashes in production --- lib/papi-dts/papi.d.ts | 6 ++++ .../server-network-connector.service.ts | 30 +++++++++++++++++-- src/shared/data/internal-connection.model.ts | 7 +++++ src/shared/services/network.service.ts | 26 +++------------- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/lib/papi-dts/papi.d.ts b/lib/papi-dts/papi.d.ts index b011d70b0d..d2786670de 100644 --- a/lib/papi-dts/papi.d.ts +++ b/lib/papi-dts/papi.d.ts @@ -603,6 +603,12 @@ declare module 'shared/data/internal-connection.model' { * to be used outside of NetworkConnectors and ConnectionService.ts */ import { ComplexRequest, ComplexResponse, SerializedRequestType } from 'shared/utils/util'; + /** + * Represents when the request router does not know to which client id the request belongs. Server + * should try to determine the correct client id through other means, and client should just send to + * server + */ + export const CLIENT_ID_UNKNOWN = -2; /** Represents when the client id has not been assigned by the server */ export const CLIENT_ID_UNASSIGNED = -1; /** "Client id" for the server */ diff --git a/src/main/services/server-network-connector.service.ts b/src/main/services/server-network-connector.service.ts index e66c5125d2..796fdf2ebb 100644 --- a/src/main/services/server-network-connector.service.ts +++ b/src/main/services/server-network-connector.service.ts @@ -1,6 +1,7 @@ import { CloseEvent, MessageEvent, WebSocket, WebSocketServer } from 'ws'; import { CLIENT_ID_SERVER, + CLIENT_ID_UNKNOWN, ConnectionStatus, InternalEvent, InternalNetworkEventHandler, @@ -19,6 +20,7 @@ import { serialize, newGuid, PlatformEventEmitter, + wait, } from 'platform-bible-utils'; import { ClientConnect, @@ -508,9 +510,31 @@ export default class ServerNetworkConnector implements INetworkConnector { throw new Error(`Received a request but cannot route it without a requestRouter`); // Figure out if we can handle this request or if we need to send it - const responderId = this.requestRouter(requestMessage.requestType); - if (this.connectorInfo.clientId === responderId) { - // This request is ours. Handle the request + let responderId = CLIENT_ID_UNKNOWN; + + // https://github.com/paranext/paranext-core/issues/51 + // If the request type doesn't have a registered handler yet, retry a few times to help with race + // conditions. This approach is hacky but works well enough for now. + const maxAttempts = 10; + const networkWaitTimeMs = 1000; + for (let attemptsRemaining = maxAttempts; attemptsRemaining > 0; attemptsRemaining--) { + responderId = this.requestRouter(requestMessage.requestType); + + if (responderId !== CLIENT_ID_UNKNOWN) break; + + logger.debug( + `Server network connector could not route request of type ${requestMessage.requestType} on attempt ${maxAttempts - attemptsRemaining + 1} of ${maxAttempts}.${attemptsRemaining === 1 ? '' : ' Retrying...'}`, + ); + + // No need to wait again after the last attempt fails + if (attemptsRemaining === 1) break; + + // eslint-disable-next-line no-await-in-loop + await wait(networkWaitTimeMs); + } + + if (this.connectorInfo.clientId === responderId || responderId === CLIENT_ID_UNKNOWN) { + // This request is ours or is unknown. Handle the request if (!this.localRequestHandler) throw new Error('Handling request without a requestHandler!'); // Run the request handler for this request diff --git a/src/shared/data/internal-connection.model.ts b/src/shared/data/internal-connection.model.ts index c22d1cf487..103ade91cd 100644 --- a/src/shared/data/internal-connection.model.ts +++ b/src/shared/data/internal-connection.model.ts @@ -5,6 +5,13 @@ import { ComplexRequest, ComplexResponse, SerializedRequestType } from '@shared/utils/util'; +/** + * Represents when the request router does not know to which client id the request belongs. Server + * should try to determine the correct client id through other means, and client should just send to + * server + */ +export const CLIENT_ID_UNKNOWN = -2; + /** Represents when the client id has not been assigned by the server */ export const CLIENT_ID_UNASSIGNED = -1; diff --git a/src/shared/services/network.service.ts b/src/shared/services/network.service.ts index 08a71f318b..efd90bfde1 100644 --- a/src/shared/services/network.service.ts +++ b/src/shared/services/network.service.ts @@ -7,18 +7,17 @@ import { ClientConnectEvent, ClientDisconnectEvent, - CLIENT_ID_SERVER, NetworkEventHandler, RequestHandler, RequestRouter, CATEGORY_COMMAND, + CLIENT_ID_UNKNOWN, } from '@shared/data/internal-connection.model'; import { aggregateUnsubscriberAsyncs, stringLength, UnsubscriberAsync, getErrorMessage, - wait, PlatformEventEmitter, PlatformEvent, indexOf, @@ -187,24 +186,7 @@ const requestRawUnsafe = async ( `Cannot perform raw request ${requestType} as the NetworkService is not initialized`, ); - // https://github.com/paranext/paranext-core/issues/51 - // If the request type doesn't have a registered handler yet, retry a few times to help with race - // conditions. This approach is hacky but works well enough for now. - const expectedErrorMsg: string = `No handler was found to process the request of type ${requestType}`; - const maxAttempts: number = 10; - for (let attemptsRemaining = maxAttempts; attemptsRemaining > 0; attemptsRemaining--) { - // eslint-disable-next-line no-await-in-loop - const response = await connectionService.request(requestType, contents); - if (response.success || attemptsRemaining === 1 || response.errorMessage !== expectedErrorMsg) - return response; - - // eslint-disable-next-line no-await-in-loop - await wait(1000); - - logger.debug(`Retrying network service request of type ${requestType}`); - } - - throw new Error(`Raw request ${requestType} failed`); + return connectionService.request(requestType, contents); }; /** @@ -652,8 +634,8 @@ const routeRequest: RequestRouter = (requestType: string): number => { const registration = requestRegistrations.get(requestType); if (!registration) // We are the client and we need to send the request to the server or we are the server and we - // need to return an error - return CLIENT_ID_SERVER; + // need to try to figure out the recipient or return an error + return CLIENT_ID_UNKNOWN; if (registration.registrationType === 'local') // We will handle this request here return connectionService.getClientId();