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

Uncaught error is thrown when port for notifier server is already in use #95

Closed
pixtron opened this issue Nov 13, 2018 · 9 comments
Closed

Comments

@pixtron
Copy link

pixtron commented Nov 13, 2018

Uncaught error is thrown when port for notifier server is already in use

Expected Behavior

The error EADDRINUSE should be caught and handled. Idealy the error would be bubbled up to the app if it can't be handled (see #94).

There are two cases this error might occur:

1.) appauth by itself already started a server on the given port (see #89)
Eventually an already created server could be closed and a new one created (this might interrupt a previously startet auth flow).
Another solution could be to change the server so it could handle mutliple auth flows.
Alternatively bubble up the error to the app, so the app can handle it.

2.) another process is already listening on the given port
Bubble up the error to the app, so it can react accordingly.

Describe the problem

Actual Behavior

The error is not caught, and may only be caught with process.on('UncaughtException', (err) => {})

Steps to reproduce the behavior

1.) Start the example electron app (see googlesamples/appauth-js-electron-sample/pull/3 with update to appauth v1.1.1)
2.) Click "Sign in"
3.) Go back to the app without completing consent screen
4.) Click "Sign in" again

Results in

Uncaught Error: listen EADDRINUSE :::8000

Environment

  • AppAuth-JS version: 1.1.1 (similar issue already in previous versions)
  • AppAuth-JS Environment: Node/Electron
@agonzalez0515
Copy link

Can the port number be configurable?

@pixtron
Copy link
Author

pixtron commented Aug 21, 2019

@agonzalez0515 the port number is already configurable, just pass the desired port number to the constructor of NodeBaseHandler.

@dpkachaudhari
Copy link

@pixtron can you provide an example of how to change the port via NodeBaseHandler. Thanks!

@anwk
Copy link

anwk commented Oct 3, 2019

Any updates?

@tikurahul
Copy link
Collaborator

Just pass in the port to the constructor of NodeBasedHandler.

@pixtron
Copy link
Author

pixtron commented Nov 12, 2019

@tikurahul what was the reason to close this issue? For me the issue still persists. The issue isn't about configuring the port, it is about allowing to handle the EADDRINUSE error when it occurs.

@tikurahul
Copy link
Collaborator

I forgot about that part. Let me reopen the issue.

@JohnyHHH
Copy link

JohnyHHH commented Apr 1, 2020

Hi, i have faced this issue as well if user click sign in again without complete the first consent screen.
May i know is this issue have any update?

@tikurahul
Copy link
Collaborator

We don't want to allow multiple concurrent auth flows.

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

6 participants