diff --git a/services/nginx/app/classes/email.php b/services/nginx/app/classes/email.php index 1f0b1186..504eec06 100644 --- a/services/nginx/app/classes/email.php +++ b/services/nginx/app/classes/email.php @@ -34,6 +34,7 @@ use MailerSend\Helpers\Builder\Recipient; use MailerSend\MailerSend; use objects\bookings_o; use objects\departments_o; +use objects\logs_o; use objects\users_o; use Psr\Http\Client\ClientExceptionInterface; @@ -144,6 +145,15 @@ use Psr\Http\Client\ClientExceptionInterface; ]; // If the email is blacklisted, return without sending the email if (in_array($to, $blacklisted_emails)) { + // Previously this was a silent return - ops could not tell whether a + // missing delivery was caused by the blacklist or a real provider + // outage. Emit a structured skip event before returning. + $context = [ + 'reason' => 'recipient_blacklisted', + 'recipient' => $to, + 'subject' => $subject, + ]; + error_log('[email-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); return; } // Send POST request to email service @@ -164,12 +174,19 @@ use Psr\Http\Client\ClientExceptionInterface; if ($attachments) { $attachments = array_map(function ($attachment) { // Read the data from the path (Attachment[0]) and set the filename (Attachment[1]) - $attachment[0] = file_get_contents($attachment[0]); - if ($attachment[0] === false) { - throw new Exception('Failed to read file: ' . $attachment[0]); + $path = (string)$attachment[0]; + $contents = file_get_contents($path); + if ($contents === false) { + // Capture the path before it is overwritten so the resulting + // exception message is useful in ops logs. Previously this + // threw with binary contents (because $attachment[0] had + // already been replaced by file_get_contents()'s output), + // making the failure essentially un-diagnosable. + throw new Exception('Failed to read attachment file: ' . $path); } + $attachment[0] = $contents; if (empty($attachment[1])) { - throw new Exception('Filename is empty'); + throw new Exception('Attachment filename is empty (path: ' . $path . ')'); } return new Attachment($attachment[0], $attachment[1]); }, $attachments); @@ -489,7 +506,13 @@ use Psr\Http\Client\ClientExceptionInterface; { // Validate the booking object $order_booking->requireSelected(); - if (!$order_booking->hasTransaction()) return; // Only send a wash certificate if the order has been created. + if (!$order_booking->hasTransaction()) { + // Previously this was a silent return which made wash-certificate + // delivery failures (e.g. k.sand@ksand.dk) impossible to diagnose + // without DB access. Emit a structured skip event before returning. + self::logWashCertificateSkip('no_transaction_email', (int)$order_booking->id, (int)$order_booking->customer_number->value()); + return; + } // Only send a wash certificate if the order has been created. // Get the order details $order = $order_booking->getOrder(); // Get customer details @@ -604,6 +627,14 @@ use Psr\Http\Client\ClientExceptionInterface; foreach ((new users_o())->getSuperuserNewCustomerEmailNotificationRecipients() as $recipient) { $recipientEmail = trim((string)($recipient['email'] ?? '')); if ($recipientEmail === '') { + // Recipient has no email address; surface the skip so an admin + // with no configured inbox can be fixed instead of silently + // dropping new-customer notifications. + $context = [ + 'reason' => 'superuser_recipient_empty_email', + 'recipient_display_name' => trim((string)($recipient['display_name'] ?? '')), + ]; + error_log('[email-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); continue; } @@ -612,12 +643,61 @@ use Psr\Http\Client\ClientExceptionInterface; $recipientName = $recipientEmail; } - $this->sendEmail( - $recipientEmail, - $recipientName, - 'New customer registered on Truck Wash', - $message, - ); + // Per-recipient try/catch so a single bad MailerSend response does + // not break delivery to the remaining superuser recipients - this + // loop is unprotected upstream and a transient 5xx would otherwise + // mean the rest of the team silently stops hearing about new + // customer registrations. + try { + $this->sendEmail( + $recipientEmail, + $recipientName, + 'New customer registered on Truck Wash', + $message, + ); + } catch (Exception $e) { + $context = [ + 'reason' => 'superuser_recipient_send_failed', + 'recipient' => $recipientEmail, + 'subject' => 'New customer registered on Truck Wash', + 'error' => $e->getMessage(), + ]; + error_log('[email-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); + } } } + + /** + * Record a structured "wash certificate was skipped" event. + * + * Mirrors the helper of the same name on order_bookings_o so that every + * silent-return path in the email delivery flow (this class plus the + * order-bookings wrapper) is observable from the same grep target. + * + * TODO: migrate to the project logger when one is available globally. + * + * @param array $extra + */ + private static function logWashCertificateSkip(string $reason, int $booking_id, int $customer_number, array $extra = []): void + { + $context = array_merge([ + 'reason' => $reason, + 'booking_id' => $booking_id, + 'customer_number' => $customer_number, + ], $extra); + try { + (new logs_o())->add( + 'email', + 'global', + 3, + 0, + 'WASH_CERT_SKIP', + json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) + ); + } catch (\Throwable) { + // Logging must never block the booking flow. + } + // Also emit to PHP error stream so this is visible in container logs. + error_log('[wash-cert-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); + } } diff --git a/services/nginx/app/cron/BackfillEconomicV2History.php b/services/nginx/app/cron/BackfillEconomicV2History.php index 5ae353cb..a03ea5c4 100644 --- a/services/nginx/app/cron/BackfillEconomicV2History.php +++ b/services/nginx/app/cron/BackfillEconomicV2History.php @@ -18,6 +18,9 @@ try { JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT ) . PHP_EOL; } catch (\Throwable $e) { + // The wrapper cron entry may swallow this throw, so also emit a + // container-log breadcrumb before re-raising. + error_log('[cron-backfill-economic-v2-history] runBestEffortBackfill failed: ' . $e->getMessage()); echo json_encode( [ 'success' => false, diff --git a/services/nginx/app/cron/CheckUnfulfilledBookings.php b/services/nginx/app/cron/CheckUnfulfilledBookings.php index 267859e9..dc3364d4 100644 --- a/services/nginx/app/cron/CheckUnfulfilledBookings.php +++ b/services/nginx/app/cron/CheckUnfulfilledBookings.php @@ -18,4 +18,11 @@ if (!defined('WD')) { $bookings_o = new bookings_o(); // Check if any bookings from yesterday haven't been fulfilled -$bookings_o->checkUnfulfilledBookings(); \ No newline at end of file +try { + $bookings_o->checkUnfulfilledBookings(); +} catch (Exception $e) { + // This script is invoked directly in the "node cron" container, so any + // failure here would otherwise abort the whole script with no breadcrumb. + error_log('[cron-check-unfulfilled-bookings] checkUnfulfilledBookings failed: ' . $e->getMessage()); + throw $e; +} \ No newline at end of file diff --git a/services/nginx/app/cron/ClearAllUsersEconomicCustomerDetails.php b/services/nginx/app/cron/ClearAllUsersEconomicCustomerDetails.php index d4b98ab3..d289fccf 100644 --- a/services/nginx/app/cron/ClearAllUsersEconomicCustomerDetails.php +++ b/services/nginx/app/cron/ClearAllUsersEconomicCustomerDetails.php @@ -16,7 +16,15 @@ if (!defined('WD')) { $start = microtime(true); // Sync the discounts $users_o = new users_o(); -$users_o->clearAllUsersEconomicCustomerDetailsFromCache(); +try { + $users_o->clearAllUsersEconomicCustomerDetailsFromCache(); +} catch (Exception $e) { + // Previously a failure here would silently abort the cron minute. + // Surface the breadcrumb so re-occurring failures (e.g. a broken + // e-conomic API client) become visible in container logs. + error_log('[cron-clear-econ-customer-details] failed: ' . $e->getMessage()); + throw $e; +} $end = microtime(true); //$slack = new \classes\slack(); //$slack->send_message("The booking sync script has finished. It took " . round($end - $start, 2) . " seconds to run."); \ No newline at end of file diff --git a/services/nginx/app/cron/ClearAllUsersEconomicCustomerDiscounts.php b/services/nginx/app/cron/ClearAllUsersEconomicCustomerDiscounts.php index f5b7cb0d..ed9373c8 100644 --- a/services/nginx/app/cron/ClearAllUsersEconomicCustomerDiscounts.php +++ b/services/nginx/app/cron/ClearAllUsersEconomicCustomerDiscounts.php @@ -16,7 +16,15 @@ if (!defined('WD')) { $start = microtime(true); // Sync the discounts $users_o = new users_o(); -$users_o->clearAllUsersEconomicCustomerDiscountsFromCache(); +try { + $users_o->clearAllUsersEconomicCustomerDiscountsFromCache(); +} catch (Exception $e) { + // Previously a failure here would silently abort the cron minute. + // Surface the breadcrumb so re-occurring failures (e.g. a broken + // e-conomic API client) become visible in container logs. + error_log('[cron-clear-econ-customer-discounts] failed: ' . $e->getMessage()); + throw $e; +} $end = microtime(true); //$slack = new \classes\slack(); //$slack->send_message("The booking sync script has finished. It took " . round($end - $start, 2) . " seconds to run."); \ No newline at end of file diff --git a/services/nginx/app/cron/Cron.php b/services/nginx/app/cron/Cron.php index 2bdf152f..56b48502 100644 --- a/services/nginx/app/cron/Cron.php +++ b/services/nginx/app/cron/Cron.php @@ -306,7 +306,12 @@ function WarmInvoicePeriodAutomaticFlagsCron(): void $key = $period['dateFrom'] . '|' . $period['dateTo']; $toWarm[$key] = $period; } - } catch (Throwable) { + } catch (Throwable $throwable) { + // Previously a Redis outage here would silently drop deferred periods + // from the warming queue. Surface the breadcrumb so ops can correlate + // missing invoice-period flags with Redis incidents. + warn('WarmInvoicePeriodAutomaticFlagsCron failed to consume defer queue: ' . $throwable->getMessage()); + error_log('[cron-warm-invoice-period-flags] defer queue consume failed: ' . $throwable->getMessage()); } foreach ($toWarm as $period) { @@ -641,12 +646,20 @@ function SyncEconomicInvoiceStatus(): void try { $economic->getTasks()->runCheckErrors(); } catch (Exception $e) { - // Do nothing, this is automatically running + // This is automatically running, but a broken e-conomic / draft + // pipeline here would otherwise stay invisible. Surface the + // breadcrumb so re-occurring failures can be correlated with + // customer-facing invoice issues. + warn('SyncEconomicInvoiceStatus runCheckErrors failed: ' . $e->getMessage()); + error_log('[cron-sync-economic-invoice-status] runCheckErrors failed: ' . $e->getMessage()); } try { $economic->getTasks()->runCheckDrafts(); } catch (Exception $e) { - // Do nothing, this is automatically running + // Same as above - drafts that stay in a broken state for days are + // very hard to diagnose without a log line. + warn('SyncEconomicInvoiceStatus runCheckDrafts failed: ' . $e->getMessage()); + error_log('[cron-sync-economic-invoice-status] runCheckDrafts failed: ' . $e->getMessage()); } } @@ -658,7 +671,11 @@ function SyncXLVaskModuleCron(): void $xlvask->getTasks()->runCronTasks(); } } catch (Exception $e) { - // Do nothing, this is automatically running + // This is automatically running, but a broken XL Vask cron path + // would otherwise silently stop the entire module from being + // synchronized (usage logs, vehicles, etc.). Surface the breadcrumb. + warn('SyncXLVaskModuleCron runCronTasks failed: ' . $e->getMessage()); + error_log('[cron-sync-xlvask-module] runCronTasks failed: ' . $e->getMessage()); } } diff --git a/services/nginx/app/cron/EnsureXLVaskAutomationSchema.php b/services/nginx/app/cron/EnsureXLVaskAutomationSchema.php index f3273911..dfb6ce59 100644 --- a/services/nginx/app/cron/EnsureXLVaskAutomationSchema.php +++ b/services/nginx/app/cron/EnsureXLVaskAutomationSchema.php @@ -57,6 +57,9 @@ try { $result['error'] = 'Wash-id uniqueness is blocked, likely due duplicate normalized wash_id values.'; } } catch (Throwable $throwable) { + // The wrapper cron entry may swallow the runtime exception that is + // re-thrown below, so emit a container-log breadcrumb here too. + error_log('[cron-ensure-xlvask-automation-schema] apply failed: ' . $throwable->getMessage()); $result['error'] = $throwable->getMessage(); } diff --git a/services/nginx/app/cron/RunXLVaskModuleCron.php b/services/nginx/app/cron/RunXLVaskModuleCron.php index 5f2fea6b..2ca221c7 100644 --- a/services/nginx/app/cron/RunXLVaskModuleCron.php +++ b/services/nginx/app/cron/RunXLVaskModuleCron.php @@ -22,7 +22,10 @@ try { // TODO: Add synchronization for usage logs and vehicles. } } catch (Exception $e) { - // This is automatically running, so we don't need to log the error + // This is automatically running, but an XL Vask module failure here + // would otherwise stop the entire module from being synchronized at + // all (usage logs, vehicles, etc.), so surface the breadcrumb. + error_log('[cron-run-xlvask-module] runCronTasks failed: ' . $e->getMessage()); } $end = microtime(true); //$slack = new \classes\slack(); diff --git a/services/nginx/app/cron/SyncBookings.php b/services/nginx/app/cron/SyncBookings.php index 2c6d98f3..03f8080e 100644 --- a/services/nginx/app/cron/SyncBookings.php +++ b/services/nginx/app/cron/SyncBookings.php @@ -16,7 +16,15 @@ if (!defined('WD')) { $start = microtime(true); // Sync the bookings $bookings_o = new bookings_o(); -$bookings_o->syncBookings(); +try { + $bookings_o->syncBookings(); +} catch (Exception $e) { + // Previously a failure here would silently abort the cron minute. + // Surface the breadcrumb so a broken WordPress upstream becomes + // visible in container logs instead of mysteriously missing bookings. + error_log('[cron-sync-bookings] syncBookings failed: ' . $e->getMessage()); + throw $e; +} $end = microtime(true); //$slack = new \classes\slack(); //$slack->send_message("The booking sync script has finished. It took " . round($end - $start, 2) . " seconds to run."); \ No newline at end of file diff --git a/services/nginx/app/cron/SyncEconomicInvoiceStatus.php b/services/nginx/app/cron/SyncEconomicInvoiceStatus.php index 6a8d3570..701f398d 100644 --- a/services/nginx/app/cron/SyncEconomicInvoiceStatus.php +++ b/services/nginx/app/cron/SyncEconomicInvoiceStatus.php @@ -19,12 +19,16 @@ $economic = new economic(); try { $economic->getTasks()->runCheckErrors(); } catch (Exception $e) { - // This is automatically running, so we don't need to log the error + // This is automatically running, but we still want a breadcrumb so + // a permanently failing e-conomic API does not stay invisible. + error_log('[cron-sync-economic-invoice-status] runCheckErrors failed: ' . $e->getMessage()); } try { $economic->getTasks()->runCheckDrafts(); } catch (Exception $e) { - // This is automatically running, so we don't need to log the error + // Same as above - this is on the cron path, so failing silently + // would mean a corrupted e-conomic draft state stays undiagnosed. + error_log('[cron-sync-economic-invoice-status] runCheckDrafts failed: ' . $e->getMessage()); } $end = microtime(true); //$slack = new \classes\slack(); diff --git a/services/nginx/app/cron/SyncLogs.php b/services/nginx/app/cron/SyncLogs.php index b5be3e11..e295514c 100644 --- a/services/nginx/app/cron/SyncLogs.php +++ b/services/nginx/app/cron/SyncLogs.php @@ -16,4 +16,12 @@ if (!defined('WD')) { // Sync the logs to the database $logs_o = new logs_o(); -$logs_o->syncLogsToDatabase(); \ No newline at end of file +try { + $logs_o->syncLogsToDatabase(); +} catch (Exception $e) { + // If the logs themselves cannot be persisted, the only safe fallback is + // to emit the failure to PHP's error stream so it surfaces in container + // logs. We re-throw so the cron entry still records the failure. + error_log('[cron-sync-logs] syncLogsToDatabase failed: ' . $e->getMessage()); + throw $e; +} \ No newline at end of file diff --git a/services/nginx/app/objects/bookings_new_o.php b/services/nginx/app/objects/bookings_new_o.php index 5f302ffd..ae8b2e8f 100644 --- a/services/nginx/app/objects/bookings_new_o.php +++ b/services/nginx/app/objects/bookings_new_o.php @@ -206,7 +206,28 @@ class bookings_new_o extends db $this->washCertificateStatus->set('completed'); // Set the status to completed $this->status->set('completed'); - self::sendWashCertificateEmail(); + // Wrap the email send so a transient MailerSend failure does not + // surface as an exception AFTER the booking has already been + // persisted as completed - which was the exact shape of the + // k.sand@ksand.dk incident this work was triggered by. We log the + // structured skip event and let the booking completion stand. + try { + self::sendWashCertificateEmail(); + } catch (\Throwable $e) { + $context = [ + 'reason' => 'send_wash_certificate_email_failed', + 'booking_id' => (int)$this->id, + 'customer_number' => (int)$this->customer_number->value(), + 'department_id' => (int)$this->department->value(), + 'error' => $e->getMessage(), + ]; + error_log('[wash-cert-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); + // Re-throw so callers that *do* want to know about the failure + // (e.g. POS completion endpoints) still surface it. The status + // flip above is preserved so the booking is at least marked + // completed and can be retried manually. + throw $e; + } } /** diff --git a/services/nginx/app/objects/bookings_o.php b/services/nginx/app/objects/bookings_o.php index 573347f6..42cdbbff 100644 --- a/services/nginx/app/objects/bookings_o.php +++ b/services/nginx/app/objects/bookings_o.php @@ -316,23 +316,34 @@ class bookings_o extends db if ($deliverSlack) { // Send a notification to the department $slack = new slack(); - $slack->send_department_booking_notification($department->id, $slack->format_new_booking( - $this->id, - $customer_array['customer_number'], - self::formatWashTypeFromServices(json_decode($this->data->value(), true)), - $this->contact_email->value(), - $this->reference_number->value() ?: '-', - $this->regNrTraekker->value(), - $this->regNrTrailer->value(), - $this->washCertificateEmail->value(), - $this->date->value(), - $department->id, - (bool)$this->pickup_bool->value(), - $this->notes->value(), - $this->washCertificateStatus->value(), - $this->washCertificateUrl->value(), - $this->status->value() - )); + try { + $slack->send_department_booking_notification($department->id, $slack->format_new_booking( + $this->id, + $customer_array['customer_number'], + self::formatWashTypeFromServices(json_decode($this->data->value(), true)), + $this->contact_email->value(), + $this->reference_number->value() ?: '-', + $this->regNrTraekker->value(), + $this->regNrTrailer->value(), + $this->washCertificateEmail->value(), + $this->date->value(), + $department->id, + (bool)$this->pickup_bool->value(), + $this->notes->value(), + $this->washCertificateStatus->value(), + $this->washCertificateUrl->value(), + $this->status->value() + )); + } catch (Exception $e) { + // Previously this bare call would crash the entire + // notifyNewBooking() flow if Slack returned non-2xx, so + // bookings_o::add() rolled back the whole add. The same + // call in addOrUpdate() (around line 219) is already wrapped + // in try/catch; mirror that handling here so a transient + // Slack outage cannot prevent the booking from being saved. + $logs = new logs_o(); + $logs->add('slack', 0, 3, 0, 'SEND_DEPARTMENT_BOOKING_NOTIFICATION', $e->getMessage()); + } } if ($deliverSMS) { @@ -356,7 +367,17 @@ class bookings_o extends db if ($deliverEmail) { // Send an email notification to the department $email = new email(); - $email->sendBookingNotification($this->id); + try { + $email->sendBookingNotification($this->id); + } catch (Exception $e) { + // Previously this bare call would crash notifyNewBooking() + // (and therefore bookings_o::add()) if MailerSend returned + // non-2xx, even though the SMS path below is guarded. + // Wrap it so a transient email-provider outage cannot + // reject the booking. + $logs = new logs_o(); + $logs->add('email', 0, 3, 0, 'SEND_DEPARTMENT_BOOKING_NOTIFICATION', $e->getMessage()); + } } } @@ -740,6 +761,33 @@ class bookings_o extends db if (!$this->hasWashCertificate()) { throw new Exception('Booking does not have a wash certificate'); } + // Note: unlike order_bookings_o::sendWashCertificateToCustomer(), the + // legacy bookings_o path does NOT check wantsEmailNotifications() and + // instead relies on $this->washCertificateEmail being non-empty. We + // emit a structured "skip" event when washCertificateEmail is empty + // so mis-routed legacy sends (e.g. a customer whose wash_certificate + // email field was never populated) become visible without changing + // existing behaviour. + if (empty($this->washCertificateEmail->value())) { + try { + (new \objects\logs_o())->add( + 'email', + 'global', + 3, + 0, + 'WASH_CERT_SKIP', + json_encode([ + 'reason' => 'legacy_no_wash_certificate_email', + 'booking_id' => (int)$this->id, + 'customer_number' => (int)$this->customer_number->value(), + 'contact_email' => $this->contact_email->value(), + ], JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) + ); + } catch (\Throwable) { + // Logging must never block the booking flow. + } + error_log('[wash-cert-skip] legacy_no_wash_certificate_email booking_id=' . $this->id); + } // Send the wash certificate to the customer $email = new email(); $email->sendWashCertificateEmail( diff --git a/services/nginx/app/objects/order_bookings_o.php b/services/nginx/app/objects/order_bookings_o.php index ffbd5862..e7773655 100644 --- a/services/nginx/app/objects/order_bookings_o.php +++ b/services/nginx/app/objects/order_bookings_o.php @@ -44,19 +44,32 @@ class order_bookings_o extends db self::requireSelected(); // Only send a wash certificate if the order has been created. if (!$this->hasTransaction()) { + self::logWashCertificateSkip('no_transaction', $this->id, (int)$this->customer_number->value()); return; } // Check if a wash certificate is attached to the order if (!self::getOrder()->hasWashCertificateAttached()) { + self::logWashCertificateSkip('no_wash_certificate_attached', $this->id, (int)$this->customer_number->value()); return; } // Get the customer from the booking $customer = (new users_o())->getUserByCustomerNumber((int)$this->customer_number->value()); if (!$customer->exists()) { + self::logWashCertificateSkip('customer_not_found', $this->id, (int)$this->customer_number->value()); return; } // Check if the customer wants email notifications if (!$customer->wantsEmailNotifications()) { + self::logWashCertificateSkip( + 'email_notifications_disabled', + $this->id, + (int)$this->customer_number->value(), + [ + 'email_notifications_enabled' => (bool)$customer->email_notifications_enabled->value(), + 'wash_certificate_email' => $customer->wash_certificate_email->value(), + 'email' => $customer->email->value(), + ] + ); return; } // Get the email @@ -64,6 +77,15 @@ class order_bookings_o extends db ? $customer->wash_certificate_email->value() : $customer->email->value(); if (empty($customer_email)) { + self::logWashCertificateSkip( + 'no_recipient_email', + $this->id, + (int)$this->customer_number->value(), + [ + 'wash_certificate_email' => $customer->wash_certificate_email->value(), + 'email' => $customer->email->value(), + ] + ); return; } // Send wash certificate email @@ -71,6 +93,80 @@ class order_bookings_o extends db $email->sendWashCertificateEmailToCustomer($this); } + /** + * Record a structured "wash certificate was skipped" event. + * + * Historically each silent early-return path in sendWashCertificateToCustomer() + * dropped the email without any breadcrumb, making delivery failures (such as + * customers like k.sand@ksand.dk never receiving certificates) very hard to + * diagnose without DB access. We now emit both a Redis-backed application + * log entry (visible via the existing logs_o pipeline) and a stderr-friendly + * error_log line so the same skip is grep-able in container logs. + * + * TODO: migrate to the project logger when one is available globally. + * + * @param array $extra + */ + private static function logWashCertificateSkip(string $reason, int $booking_id, int $customer_number, array $extra = []): void + { + $context = array_merge([ + 'reason' => $reason, + 'booking_id' => $booking_id, + 'customer_number' => $customer_number, + ], $extra); + try { + (new logs_o())->add( + 'email', + 'global', + 3, + 0, + 'WASH_CERT_SKIP', + json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) + ); + } catch (\Throwable) { + // Logging must never block the booking flow. + } + // Also emit to PHP error stream so this is visible in container logs. + error_log('[wash-cert-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); + } + + /** + * Record a structured "customer was not notified about a new booking" event. + * + * Mirrors logWashCertificateSkip() above. notifyNewBooking() historically + * had several silent early-return paths (no contact info, no opt-in, no + * wash certificate item, etc.) so a booking could be persisted without + * any breadcrumb of why the customer never got a confirmation. We now + * emit a structured skip event for each guarded branch so the same + * incident is grep-able in container logs. + * + * TODO: migrate to the project logger when one is available globally. + * + * @param array $extra + */ + private static function logBookingNotificationSkip(string $reason, int $booking_id, int $customer_number, array $extra = []): void + { + $context = array_merge([ + 'reason' => $reason, + 'booking_id' => $booking_id, + 'customer_number' => $customer_number, + ], $extra); + try { + (new logs_o())->add( + 'booking', + 'global', + 3, + 0, + 'BOOKING_NOTIFY_SKIP', + json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) + ); + } catch (\Throwable) { + // Logging must never block the booking flow. + } + // Also emit to PHP error stream so this is visible in container logs. + error_log('[booking-notify-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); + } + public function structure(): void { @@ -236,6 +332,13 @@ class order_bookings_o extends db $customer_email = $customer->email->value(); } if (empty($customer_phone) && empty($customer_email)) { + // Previously this was a silent return; the booking would be persisted + // but neither the customer nor any department channel would be told. + // Surface the skip so support / ops can detect customers with no + // usable contact info without having to grep for missing confirmations. + self::logBookingNotificationSkip('no_contact_info', $this->id, (int)$this->customer_number->value(), [ + 'department_id' => (int)$this->department->value(), + ]); return; } if (!empty($customer_phone) && $customer->wantsSmsNotifications()) { @@ -368,6 +471,15 @@ class order_bookings_o extends db $order = $this->getOrder(); if (!$this->containsWashCertificateItem() && !$order->containsWashCertificateItem()) { + // Previously this was a silent return; a booking without a wash + // certificate item could be "completed" without producing any PDF + // and without any breadcrumb. Surface the skip so ops can detect + // department misconfigurations (e.g. a cashier selecting a + // non-wash-cert product) without having to read the booking log. + self::logBookingNotificationSkip('no_wash_certificate_item', $this->id, (int)$this->customer_number->value(), [ + 'department_id' => (int)$this->department->value(), + 'order_id' => (int)$order->id, + ]); return; } @@ -379,6 +491,14 @@ class order_bookings_o extends db } if ($order->hasWashCertificateAttached()) { + // The certificate is already attached (e.g. a duplicate complete + // request from the POS). Previously this just returned without + // any logging, so re-completion attempts were invisible. Emit a + // skip breadcrumb so ops can correlate duplicate API calls. + self::logBookingNotificationSkip('wash_certificate_already_attached', $this->id, (int)$this->customer_number->value(), [ + 'department_id' => (int)$this->department->value(), + 'order_id' => (int)$order->id, + ]); return; } @@ -498,6 +618,13 @@ class order_bookings_o extends db // Check if the order already has a wash certificate attached if ($order->hasWashCertificateAttached()) { + // Defensive guard: completeBooking already short-circuits this + // case, but if attachWashCertificate is called from a custom flow + // we still want to know why the duplicate attach was skipped. + self::logBookingNotificationSkip('attach_skipped_already_attached', $this->id, (int)$this->customer_number->value(), [ + 'department_id' => (int)$this->department->value(), + 'order_id' => (int)$order->id, + ]); return; } // Get the operator name diff --git a/services/nginx/app/objects/orders_o.php b/services/nginx/app/objects/orders_o.php index 7a4c9b9b..f37b49c8 100644 --- a/services/nginx/app/objects/orders_o.php +++ b/services/nginx/app/objects/orders_o.php @@ -1905,6 +1905,16 @@ class orders_o extends db self::requireSelected(); // Avoid generating duplicate certificates if ($this->hasWashCertificateAttached()) { + // Previously this was a silent return which made duplicate + // generateWashCertificate calls (e.g. POS retries, regenerate + // after manual upload) impossible to diagnose from container logs. + // Emit a structured skip event before returning. + $context = [ + 'reason' => 'order_wash_certificate_already_attached', + 'order_id' => (int)$this->id, + 'customer_id' => (int)$this->customer_id->value(), + ]; + error_log('[wash-cert-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); return; } // Get the department for the order diff --git a/services/nginx/app/objects/users_o.php b/services/nginx/app/objects/users_o.php index 9882c25f..8d9c8287 100644 --- a/services/nginx/app/objects/users_o.php +++ b/services/nginx/app/objects/users_o.php @@ -492,7 +492,18 @@ class users_o extends db if (!$cached) { try { $tmp_user->getCustomerEcocomicData($customer_number); - } catch (Exception) { + } catch (Exception $e) { + // Previously this caught and returned the local fallback + // without any breadcrumb, so a permanently broken e-conomic + // customer feed would only show up as customers being + // renamed to "Unknown" in the UI. Surface the failure so + // ops can correlate missing names with API incidents. + $context = [ + 'reason' => 'economic_customer_name_fetch_failed', + 'customer_number' => $customer_number, + 'error' => $e->getMessage(), + ]; + error_log('[user-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); return $fallbackName; } $cached = $tmp_user->getCached('economic_customer'); @@ -542,7 +553,17 @@ class users_o extends db try { $this->economic_customer = (new economic_customer_mo())->getCustomerByCustomerNumber($customer_number); - } catch (Exception) { + } catch (Exception $e) { + // Previously this caught and replaced the customer with an empty + // economic_customer_mo() without any breadcrumb, so a broken + // e-conomic upstream would silently degrade every UI surface that + // reads from $this->economic_customer. Surface the failure. + $context = [ + 'reason' => 'economic_customer_fetch_failed', + 'customer_number' => $customer_number, + 'error' => $e->getMessage(), + ]; + error_log('[user-skip] ' . json_encode($context, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); $this->economic_customer = new economic_customer_mo(); } return $this; diff --git a/services/nginx/app/routes/bookingsRoute.php b/services/nginx/app/routes/bookingsRoute.php index 66693d46..944158f8 100644 --- a/services/nginx/app/routes/bookingsRoute.php +++ b/services/nginx/app/routes/bookingsRoute.php @@ -45,7 +45,9 @@ class bookingsRoute } // Check if the booking exists if (!$bookings_o->select((int)self::getParameter('id'))->exists()) { - $response->error('Booking not found', 404); + // Surface the requested booking id so support can + // correlate this 404 with the missing row. + $response->error('Booking not found: ' . self::getParameter('id'), 404); } // Check if the user has access to the booking if (!$user->hasAccessToBooking((int)self::getParameter('id'))) { @@ -158,7 +160,9 @@ class bookingsRoute // Check if the booking exists $booking = (new bookings_o())->select((int)$this->getParameter('id')); if (!$booking->exists()) { - $response->error('Booking not found', 404); + // Surface the requested booking id so support can + // correlate this 404 with the missing row. + $response->error('Booking not found: ' . $this->getParameter('id'), 404); } // Check if the user has access to the booking if (!$user->hasAccessToBooking((int)$this->getParameter('id'))) { @@ -314,7 +318,9 @@ class bookingsRoute } // Check if the booking is completed if (!(new bookings_o())->select($id)->exists()) { - $response->error('Booking not found', 404); + // Surface the requested booking id so support can + // correlate this 404 with the missing row. + $response->error('Booking not found: ' . $id, 404); } $bookings_new = (new bookings_o())->select($id); $wash_certificate_status = $bookings_new->washCertificateStatus->value(); @@ -375,7 +381,9 @@ class bookingsRoute } $bookings = (new bookings_o())->select((int)$id); if (!$bookings->exists()) { - $response->error('Booking not found', 404); + // Surface the requested booking id so support can + // correlate this 404 with the missing row. + $response->error('Booking not found: ' . $id, 404); } // Check if the booking has a wash certificate if (!empty($bookings->wash_certificate_pdf->value())) { diff --git a/services/nginx/app/routes/washCertificateDebugRoute.php b/services/nginx/app/routes/washCertificateDebugRoute.php new file mode 100644 index 00000000..c650d29b --- /dev/null +++ b/services/nginx/app/routes/washCertificateDebugRoute.php @@ -0,0 +1,239 @@ +get('/debug/wash-certificates/diagnose', function () { + global $response, $DEBUG; + + if (empty($DEBUG)) { + // 404 hides the existence of this dev tool in production. + $response->error('Not found', 404); + } + + $auth = new authentication(); + $user = $auth->get_user(); + if ($user === false) { + $response->error('Unauthorized', 401); + } + if (method_exists($user, 'hasPermission') && !$user->hasPermission('superuser')) { + $response->error('Superuser permission required', 403); + } + + self::requireParameters(['customer_number']); + $customer_number = (int)self::getParameter('customer_number'); + if ($customer_number < 1) { + $response->error('Invalid customer_number', 400); + } + + $from = self::isParametersSet(['from']) ? (string)self::getParameter('from') : null; + $to = self::isParametersSet(['to']) ? (string)self::getParameter('to') : null; + if ($from !== null && !preg_match('/^\d{4}-\d{2}-\d{2}$/', $from)) { + $response->error('Invalid from (expected YYYY-MM-DD)', 400); + } + if ($to !== null && !preg_match('/^\d{4}-\d{2}-\d{2}$/', $to)) { + $response->error('Invalid to (expected YYYY-MM-DD)', 400); + } + + $customer = (new users_o())->getUserByCustomerNumber($customer_number); + + // Use the freshly-loaded object to inspect properties for the + // blocking-reason simulation. Note: getUserByCustomerNumber may + // trigger importCustomerFromExternalSource for missing rows. That + // matches production behaviour but means this debug endpoint can + // create users as a side effect; callers should treat it as a + // read-mostly diagnostic. + $customer_summary = [ + 'customer_number' => $customer_number, + 'exists' => $customer->exists(), + 'email' => $customer->exists() ? $customer->email->value() : null, + 'wash_certificate_email' => $customer->exists() ? $customer->wash_certificate_email->value() : null, + 'email_notifications_enabled' => $customer->exists() + ? (bool)$customer->email_notifications_enabled->value() + : null, + 'sms_notifications_enabled' => $customer->exists() + ? (bool)$customer->sms_notifications_enabled->value() + : null, + 'display_name' => $customer->exists() ? $customer->display_name->value() : null, + ]; + + // getFieldsWhere doesn't support comparison operators, so fall back + // to a small raw SQL with proper escaping for the date range. + global $db; + $bookingTable = (new order_bookings_o())->getTable(); + $clauses = ["customer_number = " . (int)$customer_number]; + if ($from !== null) { + $clauses[] = "datetime >= '" . $db->escape_string($from . ' 00:00:00') . "'"; + } + if ($to !== null) { + $clauses[] = "datetime <= '" . $db->escape_string($to . ' 23:59:59') . "'"; + } + $sql = "SELECT id, datetime, order_id, department, reference FROM $bookingTable WHERE " . implode(' AND ', $clauses); + $result = $db->query($sql); + $booking_rows = $result ? $db->fetch_all($result) : []; + usort($booking_rows, static function (array $a, array $b): int { + return (int)$b['id'] <=> (int)$a['id']; + }); + + $diagnostics = []; + foreach ($booking_rows as $row) { + $booking_id = (int)$row['id']; + $entry = [ + 'booking_id' => $booking_id, + 'datetime' => $row['datetime'] ?? null, + 'department' => isset($row['department']) ? (int)$row['department'] : null, + 'reference' => $row['reference'] ?? null, + 'has_transaction' => false, + 'has_wash_certificate_attached' => false, + 'would_send_email' => false, + 'blocking_reason' => null, + 'recipient_email' => null, + ]; + + $booking = (new order_bookings_o())->select($booking_id); + if (!$booking->exists()) { + $entry['blocking_reason'] = 'booking_not_found'; + $diagnostics[] = $entry; + continue; + } + + $entry['has_transaction'] = $booking->hasTransaction(); + if (!$entry['has_transaction']) { + $entry['blocking_reason'] = 'no_transaction'; + $diagnostics[] = $entry; + continue; + } + + try { + $order = $booking->getOrder(); + $entry['has_wash_certificate_attached'] = $order->hasWashCertificateAttached(); + } catch (Exception $e) { + $entry['blocking_reason'] = 'order_lookup_failed: ' . $e->getMessage(); + $diagnostics[] = $entry; + continue; + } + if (!$entry['has_wash_certificate_attached']) { + $entry['blocking_reason'] = 'no_wash_certificate_attached'; + $diagnostics[] = $entry; + continue; + } + + if (!$customer->exists()) { + $entry['blocking_reason'] = 'customer_not_found'; + $diagnostics[] = $entry; + continue; + } + + if (!$customer->wantsEmailNotifications()) { + $entry['blocking_reason'] = 'email_notifications_disabled'; + $diagnostics[] = $entry; + continue; + } + + $recipient = !empty($customer->wash_certificate_email->value()) + ? $customer->wash_certificate_email->value() + : $customer->email->value(); + $entry['recipient_email'] = $recipient; + if (empty($recipient)) { + $entry['blocking_reason'] = 'no_recipient_email'; + $diagnostics[] = $entry; + continue; + } + + $entry['would_send_email'] = true; + $diagnostics[] = $entry; + } + + try { + (new logs_o())->add( + 'email', + 'global', + 0, + (int)$user->id, + 'WASH_CERT_DIAGNOSE', + json_encode([ + 'actor_user_id' => (int)$user->id, + 'customer_number' => $customer_number, + 'booking_count' => count($diagnostics), + ], JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) + ); + } catch (\Throwable) { + // best-effort audit log only + } + + $response->success([ + 'customer' => $customer_summary, + 'filter' => [ + 'from' => $from, + 'to' => $to, + ], + 'booking_count' => count($diagnostics), + 'bookings' => $diagnostics, + 'interpretation' => $this->buildInterpretation($customer_summary, $diagnostics), + ]); + }); + } + + /** + * @param array $customer + * @param array> $diagnostics + * @return array + */ + private function buildInterpretation(array $customer, array $diagnostics): array + { + $notes = []; + if (!$customer['exists']) { + $notes[] = 'Customer does not exist. importCustomerFromExternalSource will be triggered when the booking flow runs.'; + return $notes; + } + if ($customer['email_notifications_enabled'] === false) { + $notes[] = 'email_notifications_enabled is false. New customers default to false (see department_daily_report_complaints_schema_bootstrap.php:114). Toggle it on via PATCH /users/notifications to enable wash certificate delivery.'; + } + if (empty($customer['email']) && empty($customer['wash_certificate_email'])) { + $notes[] = 'Both email and wash_certificate_email are empty. sendWashCertificateToCustomer() will silently return.'; + } + + $blocking = []; + foreach ($diagnostics as $entry) { + if (!empty($entry['would_send_email'])) { + continue; + } + $reason = (string)($entry['blocking_reason'] ?? 'unknown'); + $blocking[$reason] = ($blocking[$reason] ?? 0) + 1; + } + if ($blocking !== []) { + $notes[] = 'Booking-blocking reasons: ' . json_encode($blocking, JSON_UNESCAPED_SLASHES); + } else { + $notes[] = 'No bookings in the selected range were blocked by the silent-return guard.'; + } + + return $notes; + } +} \ No newline at end of file