Closed efelem closed 1 year ago
Regarding your proposal, I would not instanciate a client in the service and I would not handle errors at that level to comply with the SRP (single responsability principle).
I have designed the service to simply act like this:
That's all. The stopReadAndWatch
is a necessary consequence to handle graceful exit.
Therefore, the service inherits the client's life cycle. If the client dies, the service dies. If the service dies, the client lives.
The reason for this design is that some applications may use the same client for multiple services.
Example:
Since a client represents the connection itself, a reconnection policy should be handled inside the client (by the author of the package if possible). Looking at the protobuf ts manual (https://github.com/timostamm/protobuf-ts/blob/master/MANUAL.md), the client instantiation represents the connection and the author decided to let the user handles the disconnection. A reconnection strategy would be:
onRequest() {
let error;
for try 1 until 3 {
try {
const client = new Client()
const service = new Service(client, ...)
service.do()
return // Success
} catch e {
error = e
log.warn(`err {e}, try {try}`}
continue // Failure, continue
}
}
return HTTPError(error)
}
In this example, the client lifecycle is linked to the request.
By doing this, you create a "lazy" connection. However, to test onRequest
, we cannot call new
because it is not possible to mock it. Instead, you can use the factory pattern, like this :
onRequest() {
let error;
for try 1 until 3 {
try {
const client = clientFactory.create()
const service = serviceFactory.create(client, ...)
service.do()
return // Success
} catch e {
error = e
log.warn(`err {e}, try {try}`}
continue // Failure, continue
}
}
return HTTPError(error)
}
Using a factory, you can mock the factory to create "mockClient" and "mockService" simply by mocking the clientFactory
with a fake implementation (since the type of clientFactory
is simply () => IClient
) (playground)!
Solves the bug creating weird behavior as soon as more than one grpc connection for log fetching was used. We changed the singleton pattern used by a factory pattern.
Potential improvement, but I don't think we need them now as the library in its current status is not usable.
The factory pattern itself does not handle disconnections from the client. However, since the LoggerAPIClient instance is created each time the factory function is called, it is possible to handle disconnections by recreating the client instance when necessary.
One approach to handling disconnections is to catch any errors thrown by the gRPC calls and recreate the client instance if an error occurs. For example, you can modify the readAndWatch method in GRPCService to catch any errors and recreate the client instance:
This code catches any errors thrown by the loggerClient.read call and recreates the client instance by calling the factory function. Then it rethrows the error so that the calling code can handle it appropriately.
With this approach, you can handle disconnections by recreating the client instance when necessary.