phoenix.js socket.connect params doesn't accept async functions (Promises)

Created on 8 Aug 2019  路  17Comments  路  Source: phoenixframework/phoenix

Environment

  • Elixir version (elixir -v): 1.8.1
  • Phoenix version (mix deps): 1.4.9
  • NodeJS version (node -v): 11.3.0
  • NPM version (npm -v): 6.4.1
  • Operating system: OS X Majave 10.14.6

Expected behavior

The following code should work with an object, a function or an async function (Promise). No params are passed on the connection when an async function (Promise) is passed.

The following are OK.

new Socket('url', params: { ... })
new Socket('url', params: () => { ... })



md5-45f558bb51636a33b8133351c18f3fd3



new Socket('url', params: aysnc () => { ... })

Actual behavior

'params' is wrapped in a closure. The closure function doesn't await functions that are async / promises.

Most helpful comment

I had the exact same problem in the project I'm working for.

And following @chrismccord suggestion at https://github.com/phoenixframework/phoenix/issues/3515#issuecomment-628192821 , I created this npm package to wrap Phoenix to allow use params as a Promise https://www.npmjs.com/package/async-params-phoenix

Basically, it is a class that inherits Socket class, just changing the connect method. checking if params is informed and if it is a Promise.

it is a basic implementation, but worked for my case (using async storage to retrieve tokens).

All 17 comments

Hi @brweber2, can you please explain why we should await? I am not a JS specialist but I believe not all APIs that accept functions generally have to accept async functions too, correct?

Think about the value of 'params'. What should it be? It should be the value returned by invoking the 0 arity function. It works correctly for a normal function, but for asynchronous functions it doesn't await for the function to return a value.

Sorry if I am being dense, but does everywhere in JS that accepts a regular

function, does it also accept an async function?

Jos茅 Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

Definitely not everywhere, but I believe that this is a case where we do want to await the value. If you use an async function then 'params' essentially has no value. I discovered this issue on accident when the function we used was async (it came from a library - Amplify JS) and params had no value. I debugged it for a little while before I figured out what was going on. In our case the async function is the only function available to get information about the logged in user so we had no choice if we wanted to use information about the logged in user when setting the value for 'params'. IMHO I think that this boils down to a case of (a) is this a callback function or (b) a function where we use the return value. In this case, the return value is used to set the value of params, the function is not a callback function. Therefore, for async functions we have to await in order to get the value.

Hopefully my explanation is making sense... if not please let me know. Thanks for all that you do!

I'm not sure the Class constructor is async/await compatible. Similar to how getters/setters aren't async compatible.

Typescript specifically says, no you can't have "await" in a constructor. I think this comes down to the semantics of initialization - initiate an object and return immediately. Leads to non deterministic issues if it was async.

Screen Shot 2019-10-20 at 1 28 25 PM

If we have a strong need for await logic in a constructor function (which correct me if I am wrong and that is not what you are asking for), a separate initializer method would be preferred.

This line is in the constructor: phoenix.js

this.params = closure(opts.params || {})

Notice that it wraps params in a "closure". Params might be a value OR might be a function. If it is a value it returns it in a 0 arity function. Then it calls the function and uses the value. The fact that this is in a constructor does make this more complicated. I believe that @snewcomer is correct that either an init function (or another pattern like a builder) would be appropriate if you want to support async functions here. So it would require a minor re-design, but for us this was a major limiting factor because we can't get information about the logged in user without calling an async function. IMHO refactoring this is worth while, but that's just my opinion. I don't have a pull request to offer, if it is something you would like I could look into providing one, but no promises because I'm actually not that familiar with the JS side of phoenix...

From what I can tell the only place that params is called is here That is really where the await is needed.... and that is outside the constructor but it would still require a refactor I think....

I don't see a specific reason for this callback to be awaitable, since the purpose of it is we asynchronously call it. I'm also not super up to speed on latest async/await 鈥撀燾an you not await inside your own function and return to us? If not, then I don't think changing every place we accept a closure to be awaitable is a burden we should take on. I'm willing to revisit this in the future, but given the churn and incremental support of async/await, I'm inclined to punt on this for now. Thanks!

Normally, I would await the value and just pass it in. But in this situation I'm providing a function so it can be invoked at later point in time. I will look into a workaround for us. Thanks.

@brweber2 have you found any workaround on this?

@xfumihiro Not a good one, this is a problem for us because in order to get the JWT for the authenticated user we have to make an async call. We ended up putting a timer in place and using the refresh token to update a global variable. It is really hacky and I feel like there must be a pretty simple solution, but I looked into it along with a few other people and if there is one, we missed it.

@chrismccord for web apps it's fine because we can get token synchronously from localStorage but not for native apps where we have to mess with AsyncStorage
@brweber2 's hack would work but yeah it's not pretty

@brweber2 I've published modified phoenix.js & @absinthe/socket packages for testing
please try and see if it's working for you

    "@xfumihiro/absinthe-socket": "0.2.1-rc1",
    "@xfumihiro/absinthe-socket-apollo-link": "0.2.1-rc2",
    "@xfumihiro/phoenix": "1.4.0-rc1",

Hi guys, I've encountered an issue where async function for computing params would be very handy. We have a requirement to use a one time token for WS authentication with a very low ttl (for security reasons). In order properly authenticate the connection we have to fetch the one time token right before connection is established. This is all good until the connection breaks and we have to reconnect. In this case we need to fetch the one time token again, but the params function makes it impossible to implement such behaviour without nasty hacks to make the process "appear" to handle async data.

You can achieve want you want under the existing API. Since you have a tight requirement on very low TTL tokens, you likely need to take control of the reconnection process entirely, since we use exponential backoff by default, with an aggressive initial step. For example, you can control the reconnection yourself with whatever backoff (or not) you require, and fetch a new token every time:

class AuthenticatedSocket {
  constructor(path, authURL){
    this.params = {}
    this.socket = new Socket(path, {params: () => this.params})
    this.socket.onError(() => {
      this.socket.disconnect() // cancel auto connection recovery
      this.scheduleReconnect()
    })
  }

  connect(){
    this.fetchOneTimeToken(token => {
      this.params["token"] = token
      this.socket.connect()
    })
  }

  // private 
  fetchOneTimeToken(callback){
    //...
    callback(token)
  }

  scheduleReconnect(){ setTimeout(() => this.connect(), 1000) }
}

If the token has a readable/known TTL, you can also use this info to only refetch if it has expired. Not impossible and not a nasty hack :) Hope that helps!

Any new on this one? I'm developing an browser extension and the token stored inside the storage is retrieved asynchronously so async/await is inevitable...

See solution above :)

I had the exact same problem in the project I'm working for.

And following @chrismccord suggestion at https://github.com/phoenixframework/phoenix/issues/3515#issuecomment-628192821 , I created this npm package to wrap Phoenix to allow use params as a Promise https://www.npmjs.com/package/async-params-phoenix

Basically, it is a class that inherits Socket class, just changing the connect method. checking if params is informed and if it is a Promise.

it is a basic implementation, but worked for my case (using async storage to retrieve tokens).

Was this page helpful?
0 / 5 - 0 ratings