WIP: CF SDK: Modified SignalWire Client with auth states - #1192
Conversation
|
| /** | ||
| * SAT Session is for the Call Fabric SDK | ||
| */ | ||
| export class SATSessionV4 extends BaseJWTSession { |
There was a problem hiding this comment.
I did understand why we need this new SATSessionV4 class...
Why is it extending BaseJWTSession and not SATSession or even JWTSession?
There was a problem hiding this comment.
Please see the definition of JWTSession. We no longer need any of those implementations such as state persisting, socket close cleanup, etc.
There was a problem hiding this comment.
I can even remove the BaseJWTSession and directly extends from BaseSession. The only thing I need from the BaseJWTSession is the this.expired getter while calling the reauthenticate API.
| public agent = process.env.SDK_PKG_AGENT! | ||
|
|
||
| constructor(public options: SATSessionOptions) { | ||
| let decodedJwt: JWTHeader = {} |
There was a problem hiding this comment.
you can remove all this if you extend the SATSession
There was a problem hiding this comment.
I can not extend SATSession either since the new SATSessionV4 does not need to extend from JWTSession.
| * Authenticate with the SignalWire Network using SAT | ||
| * @return Promise<void> | ||
| */ | ||
| override async authenticate() { |
There was a problem hiding this comment.
I believe this is the only logic different from the SATSession.
Why not implement it on the SATSession?
There was a problem hiding this comment.
Apart from this, we also do not need to override _checkTokenExpiration in the new SATSessionV4.
Similarly, the old SATSession inherits from JWTSession which has all those logic to persist the states, that does not require in the new SATSessionV4.
jpsantosbh
left a comment
jpsantosbh
left a comment
There was a problem hiding this comment.
Looks good in general,
I think you should either not create a new SATSessionV4, but if you prefer to have the custom authorize in specific class that class should extend the SATSession.
I liked the proposed new interface, but I'm not sure about having the v4 on it, but I'll defer to Product and DevEx Team on that.
|
Refactor logic has been moved to this PR: https://github.com/signalwire/signalwire-js/pull/1200 |
2 participants
Description
SignalWireV4(beta) client with two new props:authState: Used for session reconnect and/or call reattach.onAuthStateChange: Callback trigger every time the auth state changes.Type of change
Code snippets
In case of new feature or breaking changes, please include code snippets.