Cannot add async functions to server.upgrade. Causes client connection close during the authentication.

[dederomagnolo]

Is there an existing issue for this?

• [X] I've searched for any related issues and avoided creating a duplicate issue.

Description

Hello everyone, first of all, thank you for the lib and for all the attention you do on issues. I learned a lot of websockets with your project and discussions.

I have a problem on my system and i can't understand why it is happening. I searched on the web and tried a lot of stuff, but still the problem is the same. I will try to explain the minimal code:

My app is an express based websocket:

const app = express()
app.use(express.json())
app.use(bodyParser.urlencoded({ extended: false }))
app.use(cors())

const server = http.createServer(app)

const wss = new WebSocketServer({
    noServer: true
})

following the docs, I added a first approach to authentication on server.on(upgrade):

server.on('upgrade', function upgrade(request, socket, head) {
  socket.on('error', (err) => console.error(err))

  authenticateWsClient(request).then((data) => {
        const { err, req } = data
        if (err) {
            socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n')
            socket.destroy()
            return
        }

        socket.removeListener('error', (err) => console.error(err))

        wss.handleUpgrade(request, socket, head, function done(ws) {
            wss.emit('connection', ws, request)
        })
    })
})

My problem is that the function authenticateWsClient is an async function. I need to do 2 calls for my mongoDb using the mongoose find (I know it is a very simple approach to authentication, I will improve this, but for now I only checking if the client id exists on my DB). Minimal code of it:

async function authenticateWsClient(req) {
  const url = req.url
  const splittedUrl = url.split('/')

  const deviceId = splittedUrl[1]
  console.log('------verify connected client------')

  if (!deviceId ) {
    return { err: 403, status: 403, message: 'Invalid token'}
  }

  const isDeviceValid= await Device.findOne({ deviceId })

  if (!isDeviceValid) {
    return { err: 403, status: 403, message: 'Not authorized'}
  }

 const alerts = await Alert.find({ deviceId }).select(["-__v"])

 req.alerts = alerts

  console.log(`Serial key ${deviceSerialKey} authorized. Connection Opened.`)
  return { req, message: 'Authorized' }
}

It was working when I had only first find call. When I added a second find call (the one to get the alerts), it causes my clients to close the connection, even with a timeout on the client side to wait the connection to finish. The connection seems to enter in a kind of loop, I can see the client is authorized but the connection is closed before the connection is estabilished. If I remove this "alerts" call, the thing works fine, but I think I am doing something wrong on this approach or I missing something. For sure, I can see it is because the async function, but my question is: is wrong to perform async functions on my auth/server upgrade? Is there a better way to handle that if needed or should I avoid this always?

Thanks in advance

ws version

8.6.0

Node.js Version

14.5.0

System

No response

Expected result

Connection to be estabilished normally after passes to authenticate async function on server.on('upgrade')

Actual result

Connection falls into a loop and the client cannot be connected to my server

Attachments

No response

[lpinca]

For sure, I can see it is because the async function, but my question is: is wrong to perform async functions on my auth/server upgrade? Is there a better way to handle that if needed or should I avoid this always?

The approach is ok, and I can't spot the error in the code you posted. The only thing I can see is a missing handler for the case where the promise returned by authenticateWsClient() is rejected.

[dederomagnolo]

Hey @lpinca, thanks for your response. I made a few more tests and added the catch on my authenticate function. Unfortunately got no errors from my auth function and nothing from node js side but seems like it is causing the close somehow. From my clients (ESP8266 running with Arduino Websockets) I am getting an error code 1002, which means it is "protocol error". One thing I noticed running the latest tests is that this bizarre loop is only occuring when my application is deployed if I run it locally the device is able to connect once and keep it. Now I am using the Render Hosting to turn my server live. I am using the nvmrc to setup the node version on server to 14.5.0 too.

[dederomagnolo]

Also, sometimes, running locally I get an error coming from socket.on('error', (err) => console.error(err)):

Error: read ECONNRESET at TCP.onStreamRead (node:internal/stream_base_commons:217:20) { errno: -4077, code: 'ECONNRESET', syscall: 'read' }

[lpinca]

Capture the traffic and see what is written over the wire and why the client does not like it.

Also, sometimes, running locally I get an error coming from socket.on('error', (err) => console.error(err)):

It means the that the remote peer closed the connection while the other peer was writing something.

[lpinca]

I'm closing this as answered.

[lpinca]

See also this issue: https://github.com/websockets/ws/issues/2196. The author also had problems with Render hosting.