Skip to content

Commit

Permalink
Fixed dotnet data provider not waiting for settings service so crashe…
Browse files Browse the repository at this point in the history
…s in production
  • Loading branch information
tjcouch-sil committed Jun 12, 2024
1 parent 084c31a commit d8b4370
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 25 deletions.
6 changes: 6 additions & 0 deletions lib/papi-dts/papi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
30 changes: 27 additions & 3 deletions src/main/services/server-network-connector.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { CloseEvent, MessageEvent, WebSocket, WebSocketServer } from 'ws';
import {
CLIENT_ID_SERVER,
CLIENT_ID_UNKNOWN,
ConnectionStatus,
InternalEvent,
InternalNetworkEventHandler,
Expand All @@ -19,6 +20,7 @@ import {
serialize,
newGuid,
PlatformEventEmitter,
wait,
} from 'platform-bible-utils';
import {
ClientConnect,
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/shared/data/internal-connection.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
26 changes: 4 additions & 22 deletions src/shared/services/network.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -187,24 +186,7 @@ const requestRawUnsafe = async <TParam, TReturn>(
`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<TParam, TReturn>(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<TParam, TReturn>(requestType, contents);
};

/**
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit d8b4370

Please sign in to comment.