Add adsb.lol fallback support for dual-source aircraft feed

[jonnyspicer]

Summary

Implements issue #2: Configure dual-source feed with local and adsb.lol data.

This PR adds automatic fallback to adsb.lol when the local ADS-B feed is unavailable, providing resilient aircraft data coverage.

Changes

New Components

• Proxy service (proxy/server.js): Node.js service that handles data source fallback logic
• Custom tar1090 container (Dockerfile.tar1090): Extends official tar1090 with integrated proxy
• Lighttpd proxy config (docker/lighttpd-proxy.conf): Routes /data/aircraft.json through proxy
• Entrypoint script (docker/entrypoint.sh): Manages both proxy and tar1090 services

Modified Files

• docker-compose.yml: Updated tar1090 to use custom build with environment variables
• .env.example: Fixed RECEIVER_LON naming and added fallback documentation

How It Works

1. Primary source: Proxy tries to fetch from local readsb at http://127.0.0.1:80/data/aircraft.json
2. Fallback: If local fails and ADSBLOL_ENABLED=true, fetches from adsb.lol API
3. Format conversion: Converts adsb.lol JSON to readsb-compatible format
4. Source tracking: Adds X-Data-Source header indicating local or adsb.lol

Configuration

Enable fallback in .env:

ADSBLOL_ENABLED=true
ADSBLOL_RADIUS=40  # nautical miles

Testing

Verify normal operation:

docker logs tar1090 -f
# Should show: "✓ Local readsb: N aircraft"

Test fallback:

docker stop readsb
curl -I http://localhost:80/data/aircraft.json
# Should show: X-Data-Source: adsb.lol
docker start readsb

Deployment

cd /opt/tar1090-node
git pull
docker compose down
docker compose build --no-cache tar1090
docker compose up -d

Closes #2

🤖 Generated with Claude Code

[claude]

Pull Request Review: Add adsb.lol fallback support

I've conducted a thorough review of PR #7. Overall, this is a solid implementation that successfully addresses issue #2. Below are my findings organized by category.

---

✅ Strengths

1. Clear architecture: The proxy pattern is well-chosen for implementing fallback logic
2. Good documentation: The PR description and comments in code are helpful
3. Simple implementation: Uses only Node.js built-in modules (no dependencies)
4. Proper error handling: Gracefully handles failures with appropriate fallback behavior
5. Source tracking: The X-Data-Source header is a nice touch for debugging

---

🐛 Potential Bugs & Issues

Critical

1. Race condition in entrypoint.sh (docker/entrypoint.sh:8)

   • sleep 2 is not a reliable way to wait for service readiness
   • Risk: tar1090 may start before proxy is ready, causing initial failures
   • Fix: Add a proper health check loop

2. Missing error handling for cleanup trap (docker/entrypoint.sh:11-14)

   • If PROXY_PID is not set or invalid, the trap could fail silently
   • The trap doesn't wait for the proxy to actually shut down
   • Fix: Add proper PID validation and wait logic

3. Timeout not properly enforced (proxy/server.js:34-37)

   • Setting a timeout on the request doesn't guarantee it will abort after 5s
   • The timeout event handler destroys the request but doesn't reject the promise immediately
   • Fix: Wrap in a Promise.race with setTimeout to enforce timeout

High Priority

4. No validation of environment variables (proxy/server.js:4-9)

   • Invalid RECEIVER_LAT/LON (e.g., non-numeric) will result in NaN being passed to API
   • Invalid ADSBLOL_RADIUS could cause issues
   • Recommendation: Add validation and fail-fast with clear error messages on startup

5. Memory leak potential (proxy/server.js:24-31)

   • If response never ends (e.g., connection hangs), data chunks accumulate indefinitely
   • Fix: Add a maximum response size check

6. API URL hardcoded in proxy/server.js:10

   • The adsb.lol API URL is constructed with template literals but can't be changed without rebuild
   • Recommendation: Consider making the base URL configurable via environment variable for testing/future flexibility

Medium Priority

7. No request deduplication (proxy/server.js:109-112)

   • Multiple simultaneous requests will all hit the upstream services
   • For high-traffic scenarios, this could overwhelm the backend
   • Recommendation: Add simple in-memory caching with short TTL (e.g., 1-2 seconds)

8. CORS header always present (proxy/server.js:115)

   • Access-Control-Allow-Origin: * is overly permissive
   • Recommendation: Make this configurable or restrict to specific origins

---

🔒 Security Concerns

Medium Severity

1. Unvalidated external API response (proxy/server.js:43-79)

   • The code assumes adsb.lol returns well-formed data
   • Missing/malformed fields could cause undefined values or errors
   • Risk: Malformed API response could crash the proxy
   • Fix: Add schema validation or defensive checks with filter to remove invalid entries

2. No rate limiting

   • Proxy has no protection against request flooding
   • Risk: DoS vulnerability if exposed to untrusted networks
   • Recommendation: Add basic rate limiting (e.g., 10 req/sec per IP)

3. Running as root in container (Dockerfile.tar1090:3)

   • While you switch to root to install packages, there's no switch back to unprivileged user
   • Recommendation: Add USER nobody before ENTRYPOINT, or ensure the base image's default user is used

Low Severity

4. Secrets in logs (proxy/server.js:137-141)
   • API URL includes lat/lon coordinates which may be considered sensitive
   • Risk: Low, but coordinates are PII in some jurisdictions
   • Recommendation: Redact coordinates in logs or add a log level config

---

⚡ Performance Considerations

1. Sequential fallback adds latency (proxy/server.js:82-107)

   • Current design: try local (5s timeout), then try fallback (5s timeout)
   • Worst case: 10 seconds for a single request
   • Recommendation: Consider parallel health checks with preference weighting

2. JSON parsing on every request (proxy/server.js:28-30)

   • No caching of responses means full JSON parse/stringify on every request
   • Impact: High CPU usage if request rate is high
   • Recommendation: Add response caching (see bug Add adsb.lol fallback support for dual-source aircraft feed #7)

3. String concatenation in response handler (proxy/server.js:24)

   • Using += for large responses is inefficient
   • Fix: Use Buffer.concat with chunks array instead

4. Lighttpd proxy overhead (docker/lighttpd-proxy.conf:1-7)

   • Request flow: lighttpd → Node proxy → upstream
   • Alternative: Consider using lighttpd's native proxy with fallback logic in mod_magnet (Lua)
   • Note: Current approach is fine for typical load, just noting the extra hop

---

🧪 Test Coverage

Status: ❌ No tests present

Recommendations:

1. Unit tests for convertAdsbLolToReadsb()

   • Test with various adsb.lol response formats
   • Test with missing/null fields
   • Test with malformed data

2. Integration tests

   • Test fallback behavior when local feed is down
   • Test that local feed is preferred when available
   • Test timeout handling
   • Test invalid environment variable handling

3. Docker integration tests

   • Verify entrypoint script properly starts both services
   • Test cleanup/shutdown behavior
   • Test with and without adsb.lol enabled

---

📝 Code Quality & Best Practices

Good Practices ✅

• Clear variable naming
• Consistent code formatting
• Appropriate use of const/let
• Good separation of concerns

Suggestions for Improvement

1. Add JSDoc comments for main functions (proxy/server.js:12, 43, 82)
2. Extract magic numbers to constants (proxy/server.js)
3. Use more specific error types
4. Add structured logging instead of console.log
5. Missing .dockerignore file - Should exclude node_modules, .git, *.md, etc. to reduce build context

---

🔄 Suggested Changes Summary

Must Fix (Blocking Issues)

1. Replace sleep 2 with proper health check in entrypoint.sh
2. Add timeout enforcement in fetchUrl()
3. Add validation for environment variables

Should Fix (Important)

4. Add memory protection for response buffering
5. Add defensive checks for external API data
6. Switch to unprivileged user in Dockerfile
7. Fix string concatenation performance issue

Nice to Have

8. Add basic test coverage
9. Add request caching/deduplication
10. Add structured logging
11. Add .dockerignore file
12. Add JSDoc comments

---

✨ Overall Assessment

Verdict: ✅ Approve with recommendations

This PR successfully implements the requested fallback functionality and is deployable as-is for development/testing environments. The architecture is sound and the code is generally well-written.

However, I recommend addressing at least the "Must Fix" items before deploying to production, particularly:

• The entrypoint health check (critical for reliability)
• Environment variable validation (fails fast on misconfiguration)
• Timeout enforcement (prevents hanging requests)

The code would also benefit greatly from basic test coverage to prevent regressions.

Risk Level: Medium-Low (for dev), Medium (for production without fixes)

Great work on the implementation! The fallback mechanism is exactly what was needed. 🚀