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

Unexpected disconnect errors cannot be handled #438

Closed
ScriptPup opened this issue Dec 29, 2022 · 4 comments
Closed

Unexpected disconnect errors cannot be handled #438

ScriptPup opened this issue Dec 29, 2022 · 4 comments
Labels
bug semver-major Issues that need a major version bump to be released (i.e. a breaking change)
Milestone

Comments

@ScriptPup
Copy link

Bug Report

When using the on helpers from a PubSubClient, if an error occurs and the listener is disconnected, then the entire process crashes. There does not appear to be any way to catch or gracefully handle the exception.

image

Code

  const authProvider: StaticAuthProvider = new StaticAuthProvider(
    TSH.botAccount.client_id,
    TSH.botAccount.token.access_token,
    ["channel:read:redemptions"]
  );
  try {
    const userId: string = await pubSub.registerUserListener(authProvider);
    const redemptionListener: PubSubListener = await pubSub.onRedemption(
      userId,
      (message: PubSubRedemptionMessage) => {
        logger.debug({ message }, "Recieved subscribed pubsub message");
      }
    );
    logger.info("pubSub redemption listener setup");
  } catch {
    logger.error({ authProvider }, "Unable to setup redemptions listener");
  }
  logger.debug("Listening for twitch redemptions");

Expected behavior

Should have a way to bubble/handle exceptions. Ideally this would work as a handler attached to the PubSubClient. Something like

pubSub.catch((err)=>{logger.critical({err},"A critical failure occured within the PubSub client.");}

I have other components within my app, an in particular the server has to be able to serve a web page so the user can setup/correct their bot authentication information. If the entire app crashes every time there's some kind of internal failure with the PubSub client, then they can't even fix the problem.

Actual Behavior

There does not appear to be any way to handle "Disconnected unexpectedly" errors.

Environment

  • Version: @twurple/pubsub 5.3.1, @twurple/auth 5.3.1
  • Node version: v16.13.0
  • Operating system: Windows 10 / NA
@ScriptPup ScriptPup added the bug label Dec 29, 2022
@d-fischer d-fischer added the semver-major Issues that need a major version bump to be released (i.e. a breaking change) label Dec 29, 2022
@d-fischer d-fischer added this to the 6.0 milestone Dec 29, 2022
@d-fischer
Copy link
Member

Yes, I am aware that currently there is no really good way to handle most kinds of authentication errors (among other errors) relating to persistent connections such as PubSub. My plan for 6.0 is to remove all promises from event-based interfaces and replace the error conditions with new events. I don't think it makes much sense to try and patch this on while trying to keep the backwards compatibility.

You should probably use RefreshingAuthProvider with the authorization code grant flow though.

@ScriptPup
Copy link
Author

Thanks @d-fischer for the reply. I'm glad to hear it's already something you're aware of and working a solution for.
Do you happen to have any suggestions on potential ways to handle the issue in current-state?

I don't believe I can use the RefreshAuthProvider (or rather, it wouldn't provide me much benefit over what I already have). I need to be able to store and resume the authentication token across the server being shut down, since I'm building this as a standalone package users would download (kinda think streamlabs & nightbot ++)... So I'm saving the refresh token to reuse when the server starts back up. The code itself can't be re-used (unless I'm just completely missing something). I'm storing that too, but if I try to re-use it to request a new access token I just get an error that it's expired.

From what I saw, again I could have missed something, the RefreshAuthProvider is great for a server that's expected to not be shut down (or at least not often), as it will handle the OAuth token request and such itself... I wasn't able to find an example of that provider though, so maybe I'm misunderstanding how it's used.

Either way, I already setup an authentication method that auto-refreshes the OAuth before migrating to Twurple anyway (was using tmi.js before)... So it's not like it's any less effort to use the static provider. The workflow is pretty much just: [Need to use token for something?] -> [Is token valid?] (yes->do nothing)(no->use refresh_token to get a new access token and store it)

@d-fischer
Copy link
Member

Authentication is one of the best-documented parts of the whole library. In particular, the RefreshingAuthProvider has its own documentation page and is also included with a lot of detail in the example bot tutorial.

I think it is very much helpful for your use case and is very explicitly the way to work around your issue now, since it will automatically get a new token when your old token expires, and thus can't run into the ERR_BADAUTH error. It will not refresh the token when it doesn't need to (if you set it up correctly and save all token data including the expiry-related times).

@ScriptPup
Copy link
Author

Ah, I completely missed the auth page... I was looking under examples and reference 🤦‍♂️.

However, it won't help me much with the current issue... Since the problem isn't caused by expiry. The issue is with authorization. So let's say a user sets up a bot account, and then goes an authorizes. My app sees we have a bot account, and it's authorized with a token. Now then next step is going to be to listen to the chat and listen for redemptions. Let's say the user didn't setup the scope correctly, or used the wrong account, or some other mistake. What I want to be able to do is show them a message that says the authentication failed, and here are some things you can check... I actually CAN do that, but 1-2 mins later the app crashes entirely due to the uncaught exception... So now the user needs to figure out what they did wrong, fix it, launch the app, and correct the configuration within that 1-2 minute period before it crashes again.

Currently my workaround is just to have instructions on how to clear the local database that stores the token details. That done, the app won't try to login and connect to anything so they'll have time to try again. Obviously, that's not ideal.

So the RefreshingAuthProvider won't help for this particular issue. I'm already accounting for timeouts and requesting new tokens before the old ones expire... I'm actually currently a bit over-zealous on this with refreshes a little while before the expiration hits, as well as if any calls fail... Though I'll admit I haven't really tested what happens if someone manually unlinks the authorization or something. I assume it would end up broken, though I suspect the same would be true for the RefreshingAuthProvider.

All that to say though... I do see that it looks like it accounts for my use-case though, thanks for linking the page since it accepts a refresh token instead of using the app code... I can't believe I completely glossed over it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug semver-major Issues that need a major version bump to be released (i.e. a breaking change)
Projects
None yet
Development

No branches or pull requests

2 participants