w3c / aria-at-automation-driver

A WebSocket server which allows clients to observe the text enunciated by a screen reader and to simulate user input
Other
10 stars 6 forks source link

Unavoidable Crash When Connecting to the Websocket Server #16

Closed sadikassistivlabs closed 2 years ago

sadikassistivlabs commented 2 years ago

Hi all, I've been trying to get up to speed on aria-at overall, and as such was trying to follow the setup instructions for this project in the READme, but kept hitting the following crash on the Node server after my client's websocket connection connected and the server tried to send some output.

node:buffer:328
  throw new ERR_INVALID_ARG_TYPE(
  ^

TypeError [ERR_INVALID_ARG_TYPE]: The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received an instance of Object
    at new NodeError (node:internal/errors:387:5)
    at Function.from (node:buffer:328:9)
    at toBuffer (C:\Users\sadik\AppData\Roaming\npm\node_modules\@bocoup\windows-sapi-tts-engine-for-automation\node_modules\ws\lib\buffer-util.js:95:18)
    at Sender.send (C:\Users\sadik\AppData\Roaming\npm\node_modules\@bocoup\windows-sapi-tts-engine-for-automation\node_modules\ws\lib\sender.js:315:14)
    at WebSocket.send (C:\Users\sadik\AppData\Roaming\npm\node_modules\@bocoup\windows-sapi-tts-engine-for-automation\node_modules\ws\lib\websocket.js:468:18)
    at C:\Users\sadik\AppData\Roaming\npm\node_modules\@bocoup\windows-sapi-tts-engine-for-automation\lib\create-command-server.js:9:15
    at Set.forEach (<anonymous>)
    at broadcast (C:\Users\sadik\AppData\Roaming\npm\node_modules\@bocoup\windows-sapi-tts-engine-for-automation\lib\create-command-server.js:8:18)
    at Server.<anonymous> (C:\Users\sadik\AppData\Roaming\npm\node_modules\@bocoup\windows-sapi-tts-engine-for-automation\lib\cli.js:52:19)
    at Server.emit (node:events:513:28) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Oh and I was using wscat to try and connect, as follows

wscat --connect ws://the.vm.hostname.not.actually.this:4382 --subprotocol v1.aria-at.bocoup.com

Looking through the current code I was kind of confused as it seems like #10 fixed these sorts of things, until I realized it likely hasn't been published yet per https://www.npmjs.com/package/@bocoup/windows-sapi-tts-engine-for-automation

I'm not sure how publishing a release works for this, but would it be possible to make one so I could try it again? Or if there's another way for me to work around this (maybe pulling from source?) just let me know!

sadikassistivlabs commented 2 years ago

I guess maybe figuring out how to compile the C++ code might be enough for me to build from source? Not quite sure

sadikassistivlabs commented 2 years ago

I was able to get the C++ build working in Github Actions (per this run) and I'm not getting the feeling that anything else is needed to publish? (VS is needed for install still but that's a separate thing presumably)

If so, I'll aim to loop back on this shortly and

  1. Make a PR to make a workflow to publish new releases from
  2. Publish a forked package in the meantime just so I can fully try it out

If that seems alright

sadikassistivlabs commented 2 years ago

FYI the forked version I published seemed to fix the crash for me, so I think #19 and an updated version should address this

jugglinmike commented 2 years ago

Thanks for the report! I think you're right about the binaries published to npm being behind the "main" branch. I'll see about getting an updated version published next week.

sadikassistivlabs commented 2 years ago

Yup for sure. Also I made a little Github Workflow to do the publish in #19 if you want to use that.

Just needs an NPM_TOKEN set in Github Secrets (from my testing at least).

Also really love this package by the way! Feels very transformational

jugglinmike commented 2 years ago

I've published version 0.0.4 that includes a binary built with the latest changes in the repository's "main" branch. Can you let me know if that resolves your issue?

sadikassistivlabs commented 2 years ago

Oh yup thanks! I'll make a note to check later, but I think it probably must because it was fixed when I did the same with a fork.

I'll go ahead and close this for now and re-open or comment again if I'm still seeing it later