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

Ignore gone exceptions #197

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

hansottowirtz
Copy link

It's possible that an API Gateway subscription is already closed by the client when trying to publish. E.g.:

  • The client opens a connection, the connection and subscription are stored in DynamoDB.
  • The client closes the connection, which might only be handled 200ms later because of a cold start.
  • A few milliseconds after closing, the server wants to publish to a subscription. The connection is not yet deleted from DynamoDB so the query returns the closed connection.
  • The API Gateway Management API throws a "GoneException" because the connection is closed.

This PR adds a catch block that ignores a GoneException.

This will need further verification.

@reconbot
Copy link
Owner

reconbot commented Mar 6, 2022

I wonder if it's better to go ahead and delete the connection from ddb too. This patch works fine to overcome the runtime race condition that results in a needless error. I'm happy to land it if you don't want it to be a draft.

@izidormaklary
Copy link
Contributor

It would definitely make more sense to delete the connections and the subscriptions if they are gone.

.catch(async (err) => {
    if (err?.statusCode !== 410) throw err;
    await server.models.connection.delete({ id: ConnectionId });
    await server.models.subscription.delete({ id: `${ConnectionId}|${message.id}` });
    console.warn('Connection already closed at API Gateway, deleting connection and related subscription');
};

Although this still leaves the subscriptions in the ddb that are related to the connection but aren't posted to.

@reconbot
Copy link
Owner

reconbot commented Mar 7, 2022

Well the disconnect event will take care of them, and last but not least the TTL will do the final cleanup.

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

Successfully merging this pull request may close these issues.

3 participants