mqttjs / MQTT.js

The MQTT client for Node.js and the browser
Other
8.6k stars 1.42k forks source link

TransformUrl function returns a async promise instead of new url string #828

Open mxdi9i7 opened 6 years ago

mxdi9i7 commented 6 years ago

I'm trying to request for a new signed url from a server that already signs the url, the operation is async and by returning the promise I wasn't able to have the MQTT socket connected (it was never connected, went straight to offline).

mqtt.connect({
      protocal: 'ws',
      transformWsUrl: () => {
        return renewStreamUrl(endpoint, token, conversationId) //this is an async operation
      }
    }
  )

The websocket just starts to reconnect nonstop with a new url.

The expected behavior is: transformUrl callback returns a resolved promise, which in my case is the signed url, but it seems like its only returning a wrapped Promise which causes my mqtt client to reconnect nonstop because it didn't find a valid stream URL

adamlc commented 6 years ago

This would be useful. Or a callback :)

ivands commented 6 years ago

I need this feature aswell. Please add

cto-tvd-build commented 5 years ago

I need this feature as well!

Please add! 👍

maschulze commented 5 years ago

I need this feature as well!

Please add!

hoIIer commented 4 years ago

can't you just resolve the promise prior to returning?

async transformWsUrl() {
    const url = await renewStreamUrl(...);

    return url;
}
onip commented 4 years ago

code calling that function is sync, won't be able to handle a promise or an async function which is the same.

hoIIer commented 4 years ago

@onip gotcha, wondering if it's bc fundamentally this lib wasn't built with promise support and thus async-mqtt exists? Would need to refactor the code in connect/ws.js perhaps

onip commented 4 years ago

the code using transformWsUrl() and the functions around it are all sync. I've looked to provide a MR in the past and the change was too big for my spare time.

I've resolved setting up a setInterval() to always have a fresh and valid token in order to be able to return it in a sync fashion.

hoIIer commented 4 years ago

@onip ok yeah in my app I have a separate authentication jwt/refresh mechanism that runs on an interval based on token expire, then in transformWsUrl I just use the latest value..

xiaoxiangmoe commented 4 years ago

In the current situation, I used a synchronous xhr request to transformWsUrl. This does not sound like a good idea.

viridia commented 4 years ago

I need this feature as well. I've looked at the code and yes, it would be non-trivial, since all of the various functions that call it would need to be async as well.

My use case is similar to others: The URL includes a credential which has to be fetched by a REST call, and that credential may have changed in the interval since the connection was originally established.

Attempting to use a timer to keep the credential up to date is not robust - for example, what happens if the user closes the laptop lid for an hour and then re-opens it? Or the wifi signal is interrupted? The whole point of MQTT is to be able to handle intermittent connectivity scenarios like this, but any timer running in the background isn't going to be able to fetch credentials while connectivity is down - which means that as soon as you go online again, it's now a race to see whether the socket reconnects before your REST call completes.

The only truly reliable way to handle these kinds of cases is to defer the reconnect until we are sure we have a valid credential. That's not possible with the current API.

hoIIer commented 4 years ago

hey @viridia, despite my acknowledgement above and mentioning how I have a separate auth mechanism on a timer, I'm actually sitting here thinking about how to deal with the exact scenario you described: for example, what happens if the user closes the laptop lid for an hour and then re-opens it?

It's already happening in my app where if you close your laptop for > token expire, when you reopen it the mqtt client credentials are still using the expired token which leads to an error when attempting to log back in.

Not sure what the short-term solution is yet, I actually just opened gh issue w/aws iot to inquire about how they cleanup subscriptions when a connection drops... open to idea exchange. Not relying on any major change in this lib in short-term!

viridia commented 4 years ago

What would be ideal is something similar to the interceptor callback in axios.js: the configured callback either returns a promise or a plain value. If it's a promise, then the request is deferred until the promise resolves; if it's not a promise then the request executes immediately. Internally the way axios handles this is that it always wraps the result in a call to Promise.resolve(), so it's always treated as a promise either way, albeit a promise that sometimes resolves instantly.

github-actions[bot] commented 2 years ago

This is an automated message to let you know that this issue has gone 365 days without any activity. In order to ensure that we work on issues that still matter, this issue will be closed in 14 days.

If this issue is still important, you can simply comment with a "bump" to keep it open.

Thank you for your contribution.

github-actions[bot] commented 2 years ago

This issue was automatically closed due to inactivity.

xiaoxiangmoe commented 2 years ago

?

sshakyaUR commented 11 months ago

In addition to a timer to refetch tokens, I have a event listener on visibilitychange where if the visibility state is visible, I refetch tokens. Seems to solve the issue of when a user reopens their device.

shushenghong commented 4 months ago

any update?