fix(nginx): harden error handling in bash and PowerShell installers#82
Merged
Conversation
- Replace || true on ACME .so staging copy with explicit Stop-Script so a missing libnginx_acme.so fails loudly instead of silently - Add error handling to cd in Install-Nginx (bash) to give a clear message instead of relying on set -e - Replace nginx -t && systemctl start nginx with explicit if/Stop-Script so config failures produce a readable log entry - Add lsof availability check with ss fallback in Test-RunningWebServers (both .sh and .ps1) — avoids silent skip on minimal containers - Fix ACME staging copy in PS1 to use ErrorAction Stop + catch instead of SilentlyContinue - Align pacman update failure in PS1 Update-SystemPackages to Write-Log WARN (matching bash behavior) instead of Stop-Script - Add Stop-Transcript to finally block in PS1 to ensure log is flushed https://claude.ai/code/session_01QuVvMsLEmrc5iLdMM5R3up
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the Linux nginx installer scripts by making previously-silent failures explicit (especially around ACME module staging and service start), and by improving preflight port-conflict detection for minimal environments.
Changes:
- Bash: fail fast on ACME module staging issues; add explicit error handling for
cd,nginx -t, andsystemctl start. - Bash/PowerShell: add
lsofavailability check with anss-based fallback for detecting listeners on ports 80/443. - PowerShell: make ACME staging copy errors fatal via
try/catch; makepacmanupgrade failures non-fatal; ensure transcript is stopped infinally.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
nginx/nginx_installer.sh |
Tightens failure handling during build/install and adds ss fallback for port detection. |
nginx/nginx_installer.ps1 |
Aligns error handling with bash for ACME staging and port detection; improves cleanup/logging and pacman behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…olute nginx path, portable awk - Split ACME staging into an explicit existence check ([[ -f ]]/Test-Path) with a "not built" message, followed by a cp error that preserves the actual failure reason ($_.Exception.Message in PS1; generic disk/perms hint in bash) instead of always saying "not found" - Replace bare `nginx` calls with `/usr/sbin/nginx` throughout (Install-Nginx and Test-NginxInstallation in both scripts) so PATH stripping in non-interactive/root contexts cannot mask a missing binary - Rewrite ss PID extraction to use POSIX 2-arg match() + RSTART/RLENGTH instead of gawk-only 3-arg match(), making it compatible with mawk and busybox awk - Add pre-loop availability check for lsof/ss; emit a WARN and skip the port conflict check rather than silently producing an empty result when neither tool is present https://claude.ai/code/session_01QuVvMsLEmrc5iLdMM5R3up
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Zeven bugs gevonden via vergelijking van
nginx_installer.sh/.ps1(Stensel8/Scripts) met de Toolbox patch-1 branch, en door code-analyse.Fixes
nginx_installer.sh
Build-Nginx):|| truevervangen door|| Stop-Script "..."— een mislukte copy wordt nu direct zichtbaar in plaats van stil door te gaan, waarna nginx niet start omdat het.so-bestand ontbreektcdinInstall-Nginx: ontbrekende foutafhandeling toegevoegd (|| Stop-Script) zodat een mislukking een leesbare foutmelding geeft in plaats van een stilleset -eexitnginx -t && systemctl start: vervangen doorif ! nginx -t; then Stop-Script ...; fi+ apartesystemctl start ... || Stop-Script— geeft een expliciete logentry bij config-foutenlsofinTest-RunningWebServers: availability-check toegevoegd metss-fallback — op minimale containers/images zonderlsofwerd de port-check stil overgeslagennginx_installer.ps1
Build-Nginx):ErrorAction SilentlyContinue→ErrorAction Stopin eentry/catchmetStop-ScriptUpdate-SystemPackages:Stop-Script→Write-Log 'WARN'zodat het gedrag gelijk is aan de bash-variant (pacman upgrade-fouten zijn niet fataal)finally-blok:Stop-Transcripttoegevoegd zodat de log altijd correct afgesloten wordt, ook bij een geforceerde onderbrekingTest plan
bash -n nginx/nginx_installer.shTest-RunningWebServersop een systeem zonderlsofhttps://claude.ai/code/session_01QuVvMsLEmrc5iLdMM5R3up
Generated by Claude Code