fix: upload pipe detection

[nandorpal]

User description

Fixing video upload pipeline detection

Description

Fixes #2318

Issues found:

I updated the upload.sh file, which had a pipeline detection issue and got stuck in this loop

while [ ! -p ${UPLOAD_PIPE_FILE} ];
do
      echo "Waiting for ${UPLOAD_PIPE_FILE} to be created"
      sleep 1
done

Key Changes Made:

1. Added Timeout Mechanism: The script will exit if the pipe file is not detected within a specified timeout period.
2. Improved Logging: Added more detailed logging to help diagnose issues with the pipe file detection.
3. Check File Existence Before Pipe Check: Ensures the file exists before checking if it is a named pipe.

Motivation and Context

I submitted a bug report 3 weeks ago about this issue, and it seems that it wasn't fixed till now. I wanted to fix this issue as soon as possible as our testing process relied on uploading videos to an s3 bucket. I managed to isolate the issue rather quickly and managed to write a fix on my own.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Bug fix

---

Description

• Added a timeout mechanism to exit if the pipe file is not detected within a specified period.
• Improved logging to provide more detailed information for diagnostics.
• Added a function to create the named pipe if it doesn't exist, ensuring the script handles file existence checks properly.
• Ensured the file exists before checking if it is a named pipe to avoid potential issues.

---

Changes walkthrough 📝

	Relevant files
Bug fix	upload.sh Enhance upload script with timeout and improved pipe detection Video/upload.sh Added a timeout mechanism to exit if the pipe file is not detected within a specified period. Improved logging for better diagnostics. Added a function to create the named pipe if it doesn't exist. Ensured the file exists before checking if it is a named pipe. +42/-6

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[CLAassistant]

All committers have signed the CLA.

[qodo-merge-pro]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Possible Infinite Loop The loop starting at line 72 may potentially become an infinite loop if the named pipe is not created successfully within the timeout period, or if it continuously fails to be recognized as a named pipe. This could be due to external factors affecting file creation or deletion. Resource Cleanup The script should ensure that resources (like created named pipes) are cleaned up properly in case of errors or when the script exits, to avoid leaving system resources in an inconsistent state.

[qodo-merge-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Error handling	Handle failures in named pipe creation gracefully Implement a mechanism to handle the scenario where the mkfifo command fails to create the pipe, such as logging an error message and exiting the script gracefully to avoid infinite loops or unhandled errors. Video/upload.sh [64-65] -mkfifo "${UPLOAD_PIPE_FILE}" -echo "Created named pipe ${UPLOAD_PIPE_FILE}" +if ! mkfifo "${UPLOAD_PIPE_FILE}"; then + echo "Failed to create named pipe ${UPLOAD_PIPE_FILE}" + exit 1 +else + echo "Created named pipe ${UPLOAD_PIPE_FILE}" +fi Apply this suggestion Suggestion importance[1-10]: 10 Why: This suggestion is crucial as it adds error handling for the mkfifo command, preventing the script from entering an infinite loop or failing silently, which significantly improves reliability.	10
Possible bug	Ensure SE_VIDEO_UPLOAD_BATCH_CHECK is defined and numeric before use Add a check to ensure that the SE_VIDEO_UPLOAD_BATCH_CHECK variable is defined and is a numeric value before comparing it in the if condition to prevent script errors in case of undefined or non-numeric values. Video/upload.sh [115] -if [ ${#list_rclone_pid[@]} -eq ${SE_VIDEO_UPLOAD_BATCH_CHECK} ]; +if [[ ${SE_VIDEO_UPLOAD_BATCH_CHECK} =~ ^[0-9]+$ ]] && [ ${#list_rclone_pid[@]} -eq ${SE_VIDEO_UPLOAD_BATCH_CHECK} ]; Apply this suggestion Suggestion importance[1-10]: 9 Why: This suggestion adds a necessary check to prevent potential script errors due to undefined or non-numeric values, improving the robustness of the script.	9
Robustness	Add a maximum attempt limit to the named pipe creation loop to prevent potential infinite loops To prevent potential infinite loops, add a mechanism to break out of the loop if the named pipe is not successfully created within a reasonable number of attempts, instead of relying solely on the timeout. Video/upload.sh [72-96] +MAX_ATTEMPTS=5 +attempt_count=0 while true; do + if (( attempt_count++ >= MAX_ATTEMPTS )); then + echo "Maximum attempts reached, unable to create named pipe" + exit 1 + fi ... sleep 1 done Suggestion importance[1-10]: 8 Why: This suggestion adds an additional layer of robustness by ensuring the script does not enter an infinite loop, complementing the existing timeout mechanism.	8
Performance	Optimize the loop for checking and creating the named pipe to avoid busy waiting Instead of using a while loop to continuously check and create the named pipe, consider using a more efficient approach by leveraging a timeout mechanism directly in the mkfifo command if supported, or by using a single loop with a sleep delay to avoid busy waiting. Video/upload.sh [72-96] -while true; do - if [ -e "${UPLOAD_PIPE_FILE}" ]; - then - if [ -p "${UPLOAD_PIPE_FILE}" ]; - then - break - else - echo "${UPLOAD_PIPE_FILE} exists but is not a named pipe" - create_named_pipe - fi - else - create_named_pipe +if [ ! -p "${UPLOAD_PIPE_FILE}" ]; then + if [ -e "${UPLOAD_PIPE_FILE}" ]; then + echo "${UPLOAD_PIPE_FILE} exists but is not a named pipe" + rm -f "${UPLOAD_PIPE_FILE}" fi - ... - sleep 1 -done + mkfifo "${UPLOAD_PIPE_FILE}" + echo "Created named pipe ${UPLOAD_PIPE_FILE}" +fi Suggestion importance[1-10]: 7 Why: The suggestion improves performance by reducing busy waiting, but it removes the timeout mechanism which is crucial for handling cases where the pipe is not created within a reasonable time.	7

[VietND96]

Thank you, @nandorpal !