search

openid / AppAuth-JS

JavaScript client SDK for communicating with OAuth 2.0 and OpenID Connect providers.
Apache License 2.0
985 stars 161 forks source link

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

Closed pixtron closed 3 years ago

pixtron commented 5 years ago

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

agonzalez0515 commented 5 years ago

Can the port number be configurable?

pixtron commented 5 years ago

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

dpkachaudhari commented 5 years ago

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

anwk commented 5 years ago

Any updates?

tikurahul commented 4 years ago

Just pass in the port to the constructor of NodeBasedHandler.

pixtron commented 4 years ago

@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 commented 4 years ago

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

JohnyHHH commented 4 years ago

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 commented 3 years ago

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