(old) Perchè la funzione get_files_from_paths di fbchat è buggata e come correggerla

11.06.2023

Ciao a tutti, oggi vi scrivo perchè stavo usando la libreria Python fbchat. Il repository è stato dismesso il 23 settembre 2020, però è ancora utilizzabile e funziona.

Durante l'utilizzo mi sono accorto di un errore presente all'interno della libreria, nella funzione get_files_from_paths chiamata dalla funzione sendLocalFiles. La funzione esegue queste righe:

def sendLocalFiles(
        self, file_paths, message=None, thread_id=None, thread_type=ThreadType.USER
    ):
        """Send local files to a thread.

        Args:
            file_paths: Paths of files to upload and send
            message: Additional message
            thread_id: User/Group ID to send to. See :ref:`intro_threads`
            thread_type (ThreadType): See :ref:`intro_threads`

        Returns:
            :ref:`Message ID <intro_message_ids>` of the sent files

        Raises:
            FBchatException: If request failed
        """
        file_paths = require_list(file_paths)
        with get_files_from_paths(file_paths) as x:
            files = self._upload(x)
        return self._sendFiles(
            files=files, message=message, thread_id=thread_id, thread_type=thread_type
        )

Il problema risiede nella funzione get_files_from_paths che è così formata:

@contextmanager
def get_files_from_paths(filenames):
    files = []
    try:
        for filename in filenames:
            file_obj = open(filename, "rb")
            file_info = (basename(filename), file_obj, guess_type(filename)[0])
            files.append(file_info)
        yield files
    finally:
        for _, fp, _ in files:
            fp.close()

La funzione get_files_from_paths sembra essere implementata correttamente come un generatore usando il decoratore @contextmanager. Tuttavia, c'è un potenziale problema nella chiusura dei file aperti.

Nella riga che inizia con for fn, fp, ft in files:, la chiamata fp.close() viene effettuata per ogni file aperto. Tuttavia, se si verifica un'eccezione durante l'iterazione dei file, ad esempio durante la lettura o l'elaborazione dei dati, il codice salterà alla riga successiva dopo l'istruzione yield files. In questo caso, i file aperti non verranno chiusi correttamente.

Analizziamo il codice riga per riga:

@contextmanager è un decoratore fornito dal modulo contextlib che ci permette di definire un generatore come un gestore di contesto. In questo caso, il decoratore abilita la funzione get_files_from_paths per essere utilizzata in un blocco with.
get_files_from_paths prende un argomento filenames, che dovrebbe essere una lista di nomi di file.
La variabile files viene inizializzata come una lista vuota. Questa lista conterrà le informazioni sui file aperti.
Viene eseguito un ciclo for che itera su ciascun filename presente nella lista filenames.
Per ogni filename, viene eseguita la seguente operazione:
- basename(filename) restituisce il nome base del file senza il percorso completo.
- open(filename, "rb") apre il file in modalità di lettura binaria e restituisce un oggetto file.
- guess_type(filename)[0] restituisce il tipo MIME del file, prendendo solo la prima parte del risultato restituito. La funzione guess_type viene importata da fbchat.
Il risultato dell'operazione precedente viene aggiunto come una tupla alla lista files, che conterrà le informazioni sui file aperti.
Viene eseguito l'istruzione yield files, che restituisce files come valore di ritorno del generatore. Questo è il punto in cui viene "parcheggiato" il generatore e viene fornito al blocco with che lo sta utilizzando.
Dopo che il blocco with ha completato la sua esecuzione, il controllo ritorna al generatore. Viene eseguito un altro ciclo for che itera su ciascuna tupla (fn, fp, ft) presente in files.
Per ogni tupla, viene eseguita l'operazione fp.close() per chiudere il file aperto.

Per garantire che i file vengano chiusi anche in caso di eccezioni, si può utilizzare il costrutto try-finally per assicurarci che la chiusura dei file avvenga correttamente. Ecco una versione modificata della funzione che gestisce la chiusura dei file in modo sicuro:

@contextmanager
def get_files_from_paths(filenames):
    files = []
    try:
        for filename in filenames:
            file_obj = open(filename, "rb")
            file_info = (basename(filename), file_obj, guess_type(filename)[0])
            files.append(file_info)
        yield files
    finally:
        for _, fp, _ in files:
            fp.close()
            
def sendLocalFiles(
    self, file_paths, message=None, thread_id=None, thread_type=ThreadType.USER
):
    """Send local files to a thread.

    Args:
        file_paths: Paths of files to upload and send
        message: Additional message
        thread_id: User/Group ID to send to. See :ref:`intro_threads`
        thread_type (ThreadType): See :ref:`intro_threads`

    Returns:
        :ref:`Message ID ` of the sent files

    Raises:
        FBchatException: If request failed
    """
    file_paths = require_list(file_paths)
    with get_files_from_paths(file_paths) as x:
        try:
            files = self._upload(x)
        except Exception as e:
            for fn, fp, ft in x:
                fp.close()
            raise e

    return self._sendFiles(
        files=files, message=message, thread_id=thread_id, thread_type=thread_type
    )