aws-sdk-js-v3 icon indicating copy to clipboard operation
aws-sdk-js-v3 copied to clipboard

Calling .destroy() doesn't destroy the transcribe client

Open yashjais opened this issue 2 years ago • 3 comments

Checkboxes for prior research

Describe the bug

TranscribeStreamingClient from @aws-sdk/client-transcribe-streaming has a destroy method that, as per the source code, it is designed to destroy underlying resources, including sockets. However, after invoking destroy upon an existing client, it is not destroying the client and WebSocket is still sending binary messages.

SDK version number

@aws-sdk/client-transcribe-streaming@^3.145.0

Which JavaScript Runtime is this issue in?

Browser

Details of the browser/Node.js/ReactNative version

Chrome Version 105.0.5195.102 (Official Build) (arm64)

Reproduction Steps

Here's the link to the Repo. Here's minimal reproducible code:-

import React, { Component } from 'react';
import MicrophoneStream from 'microphone-stream';
import {
  TranscribeStreamingClient,
  StartStreamTranscriptionCommand,
} from '@aws-sdk/client-transcribe-streaming';

class App extends Component {
  constructor() {
    super();

    this.state = {
      isRecordingStopped: true,
    };
    this.micStream = React.createRef();
    this.speechToText = React.createRef();
    this.transcribeClient = React.createRef();
  }

  pcmEncodeChunk = (chunk) => {
    try {
      const input = MicrophoneStream.toRaw(chunk);
      let offset = 0;
      const buffer = new ArrayBuffer(input.length * 2);
      const view = new DataView(buffer);
      for (let i = 0; i < input.length; i++, offset += 2) {
        const s = Math.max(-1, Math.min(1, input[i]));
        view.setInt16(offset, s < 0 ? s * 0x8000 : s * 0x7fff, true);
      }
      return bufferFrom(buffer);
    }
    catch (err) {
      console.log('in err of pcm encode chunk', err);
    }
  };

  awsTranscribe = async (event) => {
    try {
      const _this = this;
      const MAX_AUDIO_CHUNK_SIZE = 48000;
      const audioStream = async function* () {
        for await (const chunk of event) {
          if (chunk.length <= MAX_AUDIO_CHUNK_SIZE) {
            yield {
              AudioEvent: {
                AudioChunk: _this.pcmEncodeChunk(
                  chunk,
                ),
              },
            };
          }
        }
      };
      // Enter your credentials here
      const credentials = {
        accessKeyId: '',
        secretAccessKey: '',
        sessionToken: "",
      };

      const transcribeClient = new TranscribeStreamingClient({
        region: 'us-east-1',
        credentials,
      });
      this.transcribeClient = transcribeClient;

      const command = new StartStreamTranscriptionCommand({
        LanguageCode: 'en-US',
        MediaEncoding: 'pcm',
        MediaSampleRateHertz: 44100,
        AudioStream: audioStream(),
      });

      const data = await transcribeClient.send(command);
      let speechToText = '';
      for await (const evt of data.TranscriptResultStream) {
        for (const result of evt.TranscriptEvent.Transcript.Results || []) {
          if (result.IsPartial === false) {
            const noOfResults = result.Alternatives[0].Items.length;
            for (let i = 0; i < noOfResults; i++) {
              speechToText += `${result.Alternatives[0].Items[i].Content} `;
              this.speechToText.current = speechToText;
              console.log('speech to text', speechToText);
            }
          }
        }
      }
    }
    catch (err) {
      console.log('err', err);
    }
  };

  startRecording = async () => {
    this.setState({ isRecordingStopped: false });
    const audio = await navigator.mediaDevices.getUserMedia({
      audio: true,
      video: false,
    });

    let micStream = null;
    micStream = new MicrophoneStream();
    micStream.setStream(audio);
    this.micStream = micStream;
    this.awsTranscribe(micStream);
  };

  stopRecording = () => {
    //   console.log('destroying the client');
    this.setState({ isRecordingStopped: true });
    console.log('in the stop recording');
    this.transcribeClient?.destroy();
    this.micStream = null;
  };

  render() {
    return (
      <div>
        <button onClick={this.startRecording}>Start recording</button>
        <button onClick={this.stopRecording}>Stop recording</button>
        <div>
          {!this.state.isRecordingStopped && <p>Recording in progress</p>}
        </div>
      </div>
    )
  }
}

Observed Behavior

The client is not being terminated by calling the .destroy() method. If you go to the console and see the WebSocket request, it is still sending the message after terminating the client. Attaching a screenshot of the same. Screenshot 2022-09-12 at 4 10 31 PM

Expected Behavior

The WebSocket request that is opened by the Transcribe client_(wss://transcribestreaming.us-east-1.amazonaws.com:8443/stream-transcription-websocket)_ should be closed/terminated.

Possible Solution

No response

Additional Information/Context

No response

yashjais avatar Sep 12 '22 11:09 yashjais

Hi @yashjais ,

Thanks for submitting this bug report.

This is a known issue. I see that we have a similar issue with a workaround. Can you see if that solves your problem? https://github.com/aws/aws-sdk-js-v3/issues/2428

Thanks, Ran~

RanVaknin avatar Sep 13 '22 20:09 RanVaknin

Hey @RanVaknin. The bug that you mentioned in the comment isn't the one I'm talking about. I'm facing another issue, wherein I'm not able to close the socket request after calling the .destory() method. Can you please take a look again at the issue and description? Thank you in advance!

yashjais avatar Sep 14 '22 12:09 yashjais

Hey @RanVaknin. Did you find any leads regarding this issue? Thanks!

yashjais avatar Sep 20 '22 11:09 yashjais

Hey @RanVaknin. Did you find any leads or can you please suggest any workaround for this issue?

yashjais avatar Sep 26 '22 15:09 yashjais

@yashjais ,

Hi. Sorry didnt have bandwidth to look at this yet. Will attempt to find out more this week.

Thanks, Ran

RanVaknin avatar Oct 03 '22 18:10 RanVaknin

@RanVaknin I'm having the same issue for pinpoint and cognito. I called destroy, but it didn't close the connection.

Have you had a chance to take a look? It causes a lot of issues for me.

Not closing a connection means it would be timeout at some point, and that timeout interprets my lambda instance.

jparksecurity avatar Nov 29 '22 20:11 jparksecurity

For anyone who might face this issue, I resolved it by doing some workaround. I was able to kill the WebSocket with a different technique. I'm using microphone-stream to obtain the stream of audio from the client. This package provides a method stop() to stop the recording of the microphone. Using stop() on an instance of microphone stream did kill the WebSocket for me!

yashjais avatar Dec 12 '22 06:12 yashjais

Issue

TranscribeStreamingClient.destroy() is in fact calling the config.requestHandler.destroy()

https://github.com/aws/aws-sdk-js-v3/blob/b03bc8f09c20173b39be840c83fa6fcc9e25adf8/packages/smithy-client/src/client.ts#L66

And, Surprise!, destroy method of the default requestHandler is empty.

https://github.com/aws/aws-sdk-js-v3/blob/208c93b0da339330ddcd9198f24f9016ac154be9/packages/middleware-sdk-transcribe-streaming/src/websocket-handler.ts#L29

Possible workaround

We can overwrite the default request handler like this

const client = new TranscribeStreamingClient({
    // ...
    requestHandler: new WebSocketHandler(),
});

And there is my version of https://github.com/aws/aws-sdk-js-v3/blob/208c93b0da339330ddcd9198f24f9016ac154be9/packages/middleware-sdk-transcribe-streaming/src/websocket-handler.ts but in JavaScript as my repo isn't TypeScript.

import {
  iterableToReadableStream,
  readableStreamtoIterable,
} from '@aws-sdk/eventstream-serde-browser';
import { HttpResponse } from '@aws-sdk/protocol-http';
import { formatUrl } from '@aws-sdk/util-format-url';

export class WebSocketHandler {
  constructor({ connectionTimeout } = {}) {
    this.metadata = {
      handlerProtocol: 'websocket',
    };
    this.connectionTimeout = connectionTimeout || 2000;
    this.ws;
    this.stopped;
  }
  destroy() {
    // Store stopped value so it doesn't raise an error when WebSocket is in closed state.
    this.stopped = true;
    this.ws.close(1000, 'Closed by user');
  }
  async handle(request) {
    const url = formatUrl(request);
    const socket = new WebSocket(url);
    // Store the socket to be able to close it.
    this.ws = socket;
    this.stopped = false;
    socket.binaryType = 'arraybuffer';
    await waitForReady(socket, this.connectionTimeout);
    const { body } = request;
    const bodyStream = getIterator(body);
    const asyncIterable = this.connect(socket, bodyStream);
    const outputPayload = toReadableStream(asyncIterable);
    return {
      response: new HttpResponse({
        statusCode: 200,
        body: outputPayload,
      }),
    };
  }

  connect(socket, data) {
    let streamError = undefined;
    const outputStream = {
      [Symbol.asyncIterator]: () => ({
        next: () =>
          new Promise((resolve, reject) => {
            socket.onerror = (error) => {
              socket.onclose = null;
              socket.close();
              reject(error);
            };
            socket.onclose = () => {
              if (streamError) {
                reject(streamError);
              } else {
                resolve({
                  done: true,
                  value: undefined,
                });
              }
            };
            socket.onmessage = (event) => {
              resolve({
                done: false,
                value: new Uint8Array(event.data),
              });
            };
          }),
      }),
    };
    const send = async () => {
      try {
        for await (const inputChunk of data) {
          // Only send if the WebSocket is Open or the Browser will raise an error.
          if (socket.readyState === socket.OPEN) socket.send(inputChunk);
          if (socket.readyState === socket.CLOSED) {
            // Throw an error to stop the iterations
            // Without the Error, the iterator will keep trying to send data.
            throw Error('WebSocket is already in CLOSED state.');
          }
        }
      } catch (err) {
        streamError = err;
        if (!this.stopped) {
          // Throw an error because when connection is closed,
          // the onclose event will come before and will not read streamError
          throw Error(err);
        }
      } finally {
        socket.close(1000);
      }
    };
    send();
    return outputStream;
  }
}
const waitForReady = (socket, connectionTimeout) =>
  new Promise((resolve, reject) => {
    const timeout = setTimeout(() => {
      reject({
        $metadata: {
          httpStatusCode: 500,
        },
      });
    }, connectionTimeout);
    socket.onopen = () => {
      clearTimeout(timeout);
      resolve();
    };
  });

const getIterator = (stream) => {
  if (stream[Symbol.asyncIterator]) return stream;
  else if (isReadableStream(stream)) {
    return readableStreamtoIterable(stream);
  }

  return {
    async *[Symbol.asyncIterator]() {
      yield stream;
    },
  };
};
const toReadableStream = (asyncIterable) =>
  typeof ReadableStream === 'function'
    ? iterableToReadableStream(asyncIterable)
    : asyncIterable;
const isReadableStream = (payload) =>
  typeof ReadableStream === 'function' && payload instanceof ReadableStream;

exlame avatar Jan 18 '23 16:01 exlame

+1

tbrains avatar Jan 20 '23 09:01 tbrains

We also facing the same issue.

Yadukrish avatar Jan 20 '23 15:01 Yadukrish

Hi there, I can confirm that the following comment is true, basically the body of the destroy function from the web socket handler is empty. I will mark this issue to be reviewed so it can be further addressed.

Thanks!

yenfryherrerafeliz avatar Feb 01 '23 18:02 yenfryherrerafeliz

A fix is merged in https://github.com/aws/aws-sdk-js-v3/pull/4400

It will be released with https://github.com/aws/aws-sdk-js-v3/releases/tag/v3.266.0, expected on Feb 6.

trivikr avatar Feb 04 '23 00:02 trivikr

It will be released with v3.266.0 (release), expected on Feb 6.

The fix was released with https://github.com/aws/aws-sdk-js-v3/releases/tag/v3.266.0 on Feb 6.

@yashjais Can you verify if it's fixed in your application, by bumping to v3.266.0?

trivikr avatar Feb 07 '23 18:02 trivikr

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

github-actions[bot] avatar Feb 22 '23 00:02 github-actions[bot]