Skip to content

fix: show error message for tts model params not correct#1955

Merged
liuruibin merged 1 commit intomainfrom
pr@main@fix_show_tts_err
Dec 31, 2024
Merged

fix: show error message for tts model params not correct#1955
liuruibin merged 1 commit intomainfrom
pr@main@fix_show_tts_err

Conversation

@shaohuzhang1
Copy link
Contributor

fix: show error message for tts model params not correct --bug=1051057 --user=刘瑞斌 【模型管理】语音合成模型有错误的参数,播放回答内容时没有反应 https://www.tapd.cn/57709429/s/1639429

--bug=1051057 --user=刘瑞斌 【模型管理】语音合成模型有错误的参数,播放回答内容时没有反应 https://www.tapd.cn/57709429/s/1639429
@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Dec 31, 2024

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Dec 31, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

}
// 假设我们有一个 MP3 文件的字节数组
// 创建 Blob 对象
const blob = new Blob([res], { type: 'audio/mp3' })
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code looks generally clean and well-structured for its functionalities. However, there are a few improvements and optimizations that can be suggested:

  1. Type Casting: Ensure type casting is used consistently throughout the code to avoid runtime errors.

    const id = prop.id as unknown; // Use as unknown if `prop` is not guaranteed to have an `id` property
  2. Async/Await Handling: For asynchronous operations like file fetching, it's better to explicitly handle promises and avoid relying on chaining.

    // Assuming applicationApi.postTextToSpeech returns a promise
    async function playAnswerText(text: string) {
      try {
        const response = await applicationApi.postMessageToSpeech((props.applicationId as string) || (id as string), { text });
        
        if (response.status === 400 && response.headers['content-type'] !== 'application/json') {
          MsgError('Invalid request.');
          return;
        }
    
        // Process the response depending on its content type
        if (response.data.fileUrl) {
          // Handle file URL
        } else if (response.data.mp3Data) {
          // Convert mp3 data to audio element
        }
      } catch (error) {
        console.error('Error during speech synthesis:', error);
        msgError(error.message);
      }
    }
  3. Comments and Documentation: Adding comments and documentation can help maintainability of the codebase.

    /**
     * Plays the given answer text using the speech synthesizer API.
     * Handles JSON responses differently from other types of responses.
     *
     * @param text The text to be synthesized.
     */
    async function playAnswerText(text: string) {
      // Your implementation here...
    }
  4. Error Logging: Consider adding logging mechanisms to record errors effectively, especially when dealing with network requests or unexpected behavior.

  5. Input Validation: Validate inputs before making API calls to prevent bugs caused by invalid data.

By applying these recommendations, the code will become more robust, readable, and maintainable.

}
// 假设我们有一个 MP3 文件的字节数组
// 创建 Blob 对象
const blob = new Blob([res], { type: 'audio/mp3' })
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one main issue in the provided code snippet:

  1. Potential Memory Leak with MsgError: If a network request fails or returns an unexpected response that still contains HTML/XML content, the MsgError function might attempt to parse it, leading to unnecessary memory allocations.

  2. Optimization: Since you are using await res.text() when the response type is JSON, consider checking the contentType header of the response before attempting to read body content as text. This can prevent unnecessary conversions and improve performance.

  3. Code Structure Review:

    const playAnswerText = (text: string) => {
      if (!response || !response.ok) {
        console.error('Network Error:', error);
        return;
      }
      
      switch (response.headers.get('Content-Type')) {
        case 'application/json':
          const jsonData = await response.json();
          displayJsonData(jsonData); // Your custom handling for JSON data
          break;
        default:
          downloadFileFromResponse(response);
          break;
      }
    }

### Suggested Changes:
- **Avoid Parsing Non-JSON Responses:** Before converting non-JSON responses to strings (`async res.text()`) check the response's Content-Type Header.
 
 **Updated Code Snippet:**

 ```typescript
 playAnswerText(text: string) {
   try {
     applicationApi.postTextToSpeech(
       id || props.applicationId,
       { text },
       false
     )
       .then(async (res) => {
         switch (res.status) {
           case 200:
             if (!res.headers.has('content-type') || /json$/.test(res.headers get('content-type'))) {
               const responseData = await res.json(); // Attempt to parse json even without specific MIME types.
                 ... handle json response ...
             } else {
                 ... process other content-types if needed...
             }
             // Continue normal execution after successful parsing
             ......
             
            // Downloading file from response if necessary.
            const url = window.URL.createObjectURL(new Blob([res]));
            const link = document.createElement("a");
            link.style.display = "none";
            link.href = url;
            link.download = "downloadedFile.mp3"; // Specify appropriate filename here.
            
            document.body.appendChild(link);
            link.click();

            window.URL.revokeObjectURL(url);

             break;

           case 4XX:
           case 5XX:
             const message = await res.text();
              MsgError(message);
             break;

           default:
             break; // Other status codes that you may need to handle appropriately.
         }
      
         
       })
       .catch(error => {
         console.error('Failed POST', error.message);
          MsgError(`Error ${error.message}`);
       });
     
       
   } catch (parseErr) {
     console.log(parseErr);
      MsgError(`Parse Error: ${parseErr}`);

Summary:

The suggested changes address the memory leak risk by checking and dealing with non-json responses carefully, preventing unnecessary attempts at parsing them into text. Additionally, proper logging and clear separation between error states are maintained throughout the processing flow.

@liuruibin liuruibin merged commit 06f591d into main Dec 31, 2024
4 checks passed
@liuruibin liuruibin deleted the pr@main@fix_show_tts_err branch December 31, 2024 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants