From 6d8ea60630a89063d71ebccba9e4afcefc0f150e Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Date: Fri, 3 Jul 2026 22:22:22 +0100
Subject: [PATCH] Bug 42993: EDI: log transport failures via Koha::Logger
 instead of carp

Koha::Edifact::Transport reported every transport failure (no transport
configured, connect failure, directory-change failure, file-listing
failure and per-file download/upload failure) with carp. Under
edi_cron.pl these go to STDERR, so on a normal cron run the messages are
lost and EDI simply goes quiet with nothing in editrace.log to explain
why.

Route these failures through Koha::Logger on the 'edi' interface (the
same interface edi_cron.pl already uses) so they are recorded in the EDI
log. The two ingest()-level data warnings (duplicate filename, unreadable
file) are intentionally left as carp: one is asserted by the existing
unit test and neither is a transport failure.

No change to the transfer logic.

Test plan:
1) prove t/db_dependent/Koha/Edifact/Transport.t
2) Point an EDI account at an unreachable transport and run edi_cron.pl;
   confirm the failure is now written to the edi log rather than only to
   STDERR.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
---
 Koha/Edifact/Transport.pm | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/Koha/Edifact/Transport.pm b/Koha/Edifact/Transport.pm
index 7ebc00a37b..f98e15874a 100644
--- a/Koha/Edifact/Transport.pm
+++ b/Koha/Edifact/Transport.pm
@@ -28,6 +28,7 @@ use File::Slurp qw( read_file );
 use Koha::Database;
 use Koha::DateUtils qw( dt_from_string );
 use Koha::File::Transports;
+use Koha::Logger;
 
 sub new {
     my ( $class, $account_id ) = @_;
@@ -42,8 +43,9 @@ sub new {
         account        => $acct,
         schema         => $schema,
         file_transport => $file_transport,
-        working_dir    => C4::Context::temporary_directory,    #temporary work directory
+        working_dir    => C4::Context::temporary_directory,              #temporary work directory
         transfer_date  => dt_from_string(),
+        logger         => Koha::Logger->get( { interface => 'edi' } ),
     };
 
     bless $self, $class;
@@ -63,7 +65,7 @@ sub download_messages {
     $self->{message_type} = $message_type;
 
     unless ( $self->{file_transport} ) {
-        carp "No file transport configured for EDI account " . $self->{account}->id;
+        $self->{logger}->error( "No file transport configured for EDI account " . $self->{account}->id );
         return;
     }
 
@@ -73,21 +75,21 @@ sub download_messages {
 
     # Connect to the transport
     unless ( $self->{file_transport}->connect() ) {
-        carp "Failed to connect to file transport: " . $self->{file_transport}->id;
+        $self->{logger}->error( "Failed to connect to file transport: " . $self->{file_transport}->id );
         return;
     }
 
     # Change to download directory
     my $download_dir = $self->{file_transport}->download_directory;
     if ( $download_dir && !$self->{file_transport}->change_directory($download_dir) ) {
-        carp "Failed to change to download directory: $download_dir";
+        $self->{logger}->error("Failed to change to download directory: $download_dir");
         return;
     }
 
     # Get file list
     my $file_list = $self->{file_transport}->list_files();
     unless ($file_list) {
-        carp "Failed to get file list from transport";
+        $self->{logger}->error("Failed to get file list from transport");
         return;
     }
 
@@ -109,7 +111,7 @@ sub download_messages {
                 # Mark file as processed using the transport's rename functionality
                 $self->{file_transport}->rename_file( $filename, $processed_name );
             } else {
-                carp "Failed to download file: $filename";
+                $self->{logger}->error("Failed to download file: $filename");
             }
         }
     }
@@ -131,20 +133,20 @@ sub upload_messages {
     }
 
     unless ( $self->{file_transport} ) {
-        carp "No file transport configured for EDI account " . $self->{account}->id;
+        $self->{logger}->error( "No file transport configured for EDI account " . $self->{account}->id );
         return;
     }
 
     # Connect to the transport
     unless ( $self->{file_transport}->connect() ) {
-        carp "Failed to connect to file transport: " . $self->{file_transport}->id;
+        $self->{logger}->error( "Failed to connect to file transport: " . $self->{file_transport}->id );
         return;
     }
 
     # Change to upload directory
     my $upload_dir = $self->{file_transport}->upload_directory;
     if ( $upload_dir && !$self->{file_transport}->change_directory($upload_dir) ) {
-        carp "Failed to change to upload directory: $upload_dir";
+        $self->{logger}->error("Failed to change to upload directory: $upload_dir");
         return;
     }
 
@@ -165,13 +167,13 @@ sub upload_messages {
                     $m->status('sent');
                     $m->update;
                 } else {
-                    carp "Failed to upload file: " . $m->filename;
+                    $self->{logger}->error( "Failed to upload file: " . $m->filename );
                 }
 
                 # Clean up temp file
                 unlink $temp_file;
             } else {
-                carp "Could not create temporary file for upload: " . $m->filename;
+                $self->{logger}->error( "Could not create temporary file for upload: " . $m->filename );
             }
         }
     }