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

consider: avoid retaining port in PenumbraClient #2021

Open
turbocrime opened this issue Feb 5, 2025 · 0 comments
Open

consider: avoid retaining port in PenumbraClient #2021

turbocrime opened this issue Feb 5, 2025 · 0 comments
Assignees

Comments

@turbocrime
Copy link
Collaborator

turbocrime commented Feb 5, 2025

PenumbraClient also contains logic for reusing its port. provider refactor requires this to be reconsidered

/** Attempt to connect to the attached provider. If this client is unattached,
* a provider may be specified at this moment.
*
* May reject with an enumerated `PenumbraRequestFailure`.
*
* The public `connected` field will report the provider's connected state, or
* `undefined` if this client is not attached to a provider. The public
* `transport` field can confirm the client possesses an active connection.
*
* If called again while already connected, `connect` is a no-op.
*/
public async connect(providerOrigin?: string): Promise<void> {
if (providerOrigin) {
await this.attach(providerOrigin);
}
this.connection ??= this.createConnection();
// Connection timeouts, provider detachments, etc. appear similar to a connection denial.
// Explicitly handle denial errors and propagate them back to the caller. Without this,
// a denied connection would immediately trigger an attempt to re-establish the connection.
try {
await this.connection.port;
} catch (error) {
if (error instanceof Error && error.cause) {
if (error.cause === PenumbraRequestFailure.Denied) {
throw error;
}
}
// Clean up dangling connection resources and attempt to establish reconnection
this.destroyConnection();
this.connection = this.createConnection();
await this.connection.port;
}
}

private assertConnected(): PenumbraClientConnection {
assertProviderConnected(this.attached?.origin);
this.connection ??= this.createConnection();
return this.connection;
}

/** Request a connection to the attached provider, and return the pending
* `MessagePort` and created `Transport` without waiting. */
private createConnection(): PenumbraClientConnection {
const { confirmManifest, provider } = this.assertAttached();
// requestPort will resolve to the provider's message port if connection
// is successful. this promise is not awaited so that this method may be
// called synchronously.
const requestPort: Promise<MessagePort> = confirmManifest.then(async () => {
if (isLegacyProvider(provider)) {
await provider.request();
}
return provider.connect();
});
const connection: PenumbraClientConnection = {
port: requestPort,
transport: createChannelTransport({
...this.options.transportOptions,
getPort: () => requestPort,
}),
};
return connection;
}

@turbocrime turbocrime self-assigned this Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant