diff --git a/Makefile b/Makefile index 0d5389c..ebf000e 100644 --- a/Makefile +++ b/Makefile @@ -7,42 +7,44 @@ export ENVIRONMENT = $(ENV) # chmod 600 (not the 644 elsewhere in deploy/ — those get read by containers # under a different UID; this one is only ever read by the `docker compose` # CLI on the host, as $(ENV) invokes it, so there's no reason it needs to be -# world-readable) — and every target below removes it on the way out (see -# each recipe's `; rc=$$?; rm -f ...; exit $$rc`) rather than leaving a -# plaintext copy of every API key and both DB passwords sitting in the repo -# root after every `make` invocation. Make has no built-in "on exit" -# hook across arbitrary targets, so this is repeated per-target rather than -# centralized; `$$rc`/`exit $$rc` preserves the underlying command's exit -# code through the cleanup so a real failure (e.g. deploy.sh erroring) still -# fails the `make` invocation. +# world-readable) — and every target below removes it on the way out via a +# shell EXIT trap (same idiom deploy.sh/backup.sh already use for their own +# temp files), not a `; rm -f ...` tacked onto the end of the command. A +# real Ctrl+C on a long-running target (`logs -f`, an interactive `shell`) +# sends SIGINT to the recipe's whole foreground process group, which +# terminates a plain `cmd; rm -f ...` chain before the `;` ever runs — the +# EXIT trap fires regardless of *how* the shell exits (falls through, errors, +# or is killed by a signal), and doesn't touch $? either, so the underlying +# command's real exit code still propagates to `make`. COMPOSE_ENV_FILE := .env.$(ENV).compose $(shell grep -Ev '^(DB_PASSWORD|DB_ROOT_PASSWORD)=' .env.$(ENV) > $(COMPOSE_ENV_FILE) 2>/dev/null; chmod 600 $(COMPOSE_ENV_FILE) 2>/dev/null) COMPOSE = docker compose -p bookstore-$(ENV) -f docker-compose.yml -f docker-compose.$(ENV).yml --env-file $(COMPOSE_ENV_FILE) +CLEANUP = trap 'rm -f $(COMPOSE_ENV_FILE)' EXIT; .PHONY: up down ps logs shell wp deploy backup up: - $(COMPOSE) up -d --build; rc=$$?; rm -f $(COMPOSE_ENV_FILE); exit $$rc + $(CLEANUP) $(COMPOSE) up -d --build down: - $(COMPOSE) down; rc=$$?; rm -f $(COMPOSE_ENV_FILE); exit $$rc + $(CLEANUP) $(COMPOSE) down ps: - $(COMPOSE) ps; rc=$$?; rm -f $(COMPOSE_ENV_FILE); exit $$rc + $(CLEANUP) $(COMPOSE) ps logs: - $(COMPOSE) logs -f; rc=$$?; rm -f $(COMPOSE_ENV_FILE); exit $$rc + $(CLEANUP) $(COMPOSE) logs -f # add -u root yourself for one-off root debugging (installing a package, etc.) shell: - $(COMPOSE) exec -u www-data wordpress bash; rc=$$?; rm -f $(COMPOSE_ENV_FILE); exit $$rc + $(CLEANUP) $(COMPOSE) exec -u www-data wordpress bash # make wp ENV=staging ARGS="plugin list" wp: - $(COMPOSE) exec -u www-data wordpress wp $(ARGS); rc=$$?; rm -f $(COMPOSE_ENV_FILE); exit $$rc + $(CLEANUP) $(COMPOSE) exec -u www-data wordpress wp $(ARGS) deploy: - ./deploy/deploy.sh $(ENV); rc=$$?; rm -f $(COMPOSE_ENV_FILE); exit $$rc + $(CLEANUP) ./deploy/deploy.sh $(ENV) backup: - ./deploy/backup.sh $(ENV); rc=$$?; rm -f $(COMPOSE_ENV_FILE); exit $$rc + $(CLEANUP) ./deploy/backup.sh $(ENV) diff --git a/docker-compose.yml b/docker-compose.yml index ceda43f..65e62ab 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -108,11 +108,24 @@ services: # ready — a slow first boot (official image copying core files in, could # be worse on modest hardware) could let Caddy serve a transient 502 # before php-fpm was actually listening. + # + # cron and caddy now gate on this via depends_on: condition: + # service_healthy, which makes `docker compose up -d` (and so + # deploy.sh, under set -euo pipefail) hard-fail if wordpress never + # reports healthy in time — a new way for a deploy to fail that didn't + # exist before this healthcheck did. Budgeted generously (~4.5 min: + # start_period + retries*interval) for the 2-core/8GB box this + # actually runs on: deploy.sh's own wp-settings.php wait loop already + # budgets up to 120s just for the image's core-file copy step (chosen + # for this same hardware), and this check additionally waits for + # php-fpm itself to start, plus whatever `--build` is competing for on + # a 2-core box. A too-tight budget here would turn "slow but fine" into + # a spurious deploy failure, not just a slower one. test: ["CMD-SHELL", "bash -c '(exec 3<>/dev/tcp/127.0.0.1/9000)' 2>/dev/null"] interval: 5s timeout: 3s - retries: 20 - start_period: 30s + retries: 40 + start_period: 60s cron: build: diff --git a/wp-content/plugins/bookstore-core/includes/Cli/Commands.php b/wp-content/plugins/bookstore-core/includes/Cli/Commands.php index 09bf9b6..af0cd44 100644 --- a/wp-content/plugins/bookstore-core/includes/Cli/Commands.php +++ b/wp-content/plugins/bookstore-core/includes/Cli/Commands.php @@ -79,7 +79,16 @@ class Commands $result = SyntheticOfferGenerator::generate_all(500, function (int $processed) { \WP_CLI::log(" generated offers for {$processed} ISBNs..."); }); - \WP_CLI::success("Generated offers for {$result['processed']} ISBNs."); + + foreach ($result['failed'] as $failure) { + \WP_CLI::warning($failure); + } + + \WP_CLI::success(sprintf( + 'Generated offers for %d ISBNs (%d failed).', + $result['processed'], + count($result['failed']) + )); } /** diff --git a/wp-content/plugins/bookstore-core/includes/Import/SyntheticOfferGenerator.php b/wp-content/plugins/bookstore-core/includes/Import/SyntheticOfferGenerator.php index d2b63b4..ffd479c 100644 --- a/wp-content/plugins/bookstore-core/includes/Import/SyntheticOfferGenerator.php +++ b/wp-content/plugins/bookstore-core/includes/Import/SyntheticOfferGenerator.php @@ -45,11 +45,12 @@ class SyntheticOfferGenerator /** * @param callable|null $on_progress function(int $processed): void - * @return array{processed:int} + * @return array{processed:int, failed:string[]} */ public static function generate_all(int $batch_size = 500, ?callable $on_progress = null): array { $processed = 0; + $failed = []; $offset = 0; while (true) { $isbns = Isbn::all_paginated($batch_size, $offset); @@ -57,7 +58,18 @@ class SyntheticOfferGenerator break; } foreach ($isbns as $isbn13) { - self::generate_for_isbn($isbn13); + // SupplierOffer::insert() throws on a real DB failure (fixed + // elsewhere this session, matching Work/Edition/Isbn) — that + // fix's whole point is making a bad insert visible instead of + // silent, but visible must not mean "abort the entire batch." + // One bad ISBN (a transient deadlock, a connection blip) + // shouldn't cost every other ISBN's offer in the run, same + // reasoning as GutenbergImporter's per-row catch. + try { + self::generate_for_isbn($isbn13); + } catch (\RuntimeException $e) { + $failed[] = "{$isbn13}: " . $e->getMessage(); + } $processed++; } if ($on_progress) { @@ -65,6 +77,6 @@ class SyntheticOfferGenerator } $offset += $batch_size; } - return ['processed' => $processed]; + return ['processed' => $processed, 'failed' => $failed]; } } diff --git a/wp-content/plugins/bookstore-core/includes/Product/ProductSync.php b/wp-content/plugins/bookstore-core/includes/Product/ProductSync.php index 4009ef7..8c4cac9 100644 --- a/wp-content/plugins/bookstore-core/includes/Product/ProductSync.php +++ b/wp-content/plugins/bookstore-core/includes/Product/ProductSync.php @@ -163,6 +163,17 @@ class ProductSync } $product_id = $product->save(); + if (!$product_id) { + // WC_Product::save() returns 0 rather than throwing on a + // wp_insert_post()-level rejection (confirmed directly via the + // wp_insert_post_empty_content filter). Left unchecked, this + // work_id gets Work::set_product_id($work_id, 0) — silently + // "succeeds," isn't visible in $failed[], and every other + // failure mode in this method already goes through the + // per-product SAVEPOINT + $failed[] reporting. Throwing here + // routes it through the same path instead of a special case. + throw new \RuntimeException("WC_Product::save() returned no product ID (work_id: {$work->work_id})"); + } update_post_meta($product_id, '_bsc_work_id', $work->work_id); // Unconditional, not "if new": $existing_id can be a stale pointer to