Feature/freshdesk contact - #32
Conversation
mbwatson
left a comment
mbwatson
left a comment
There was a problem hiding this comment.
looks good. one tiny change, and it's good to go!
There was a problem hiding this comment.
oops duplicate import here, but that was here before. might be nice to clean up now.
- Added shared services doc: services/README.md
- explains one services/.env for both local Python services
- includes startup commands for both ports 8787 and 8788
- includes apps/site/.env wiring (FRESHDESK_PROXY_URL, FRESHDESK_JOIN_URL)
- Updated env loading in both local servers to support a shared env file:
- services/freshdesk/server.py
- services/freshdesk-join/server.py
- load order is now:
1. services/.env (shared)
2. optional service-specific override (services/freshdesk/.env or services/freshdesk-join/.env)
- Updated docs in each service to match shared-env workflow:
- services/freshdesk/README.md
- services/freshdesk-join/README.md
- Setup shared .env and added shared sample template:
- services/sample.env
- Removed duplicate per-service env templates:
- deleted services/freshdesk/sample.env
- deleted services/freshdesk-join/sample.env
- Renamed services/freshdesk → services/freshdesk-proxy - Updated path references in docs: - README.md:117 - services/README.md:23 - services/freshdesk-proxy/README.md:27 - services/freshdesk-join/README.md:5 - Updated code comments that pointed to old path: - apps/site/src/components/forms/DynamicForm.tsx:194 - apps/site/src/components/forms/DynamicCustomObjectForm.tsx:179 - Removed moved Python cache dirs so they don’t linger under the new folder.
Sjk freshdesk contact
There was a problem hiding this comment.
ok, getting closer!
i see a potential unhandled exception around reCAPTCHA verification in both services/freshdesk-proxy/handler.py and services/freshdesk-join/handler.py that could be an issue:
in fresdhdesk-join/handler.py:
verification = verify_recaptcha(recaptcha_token)on line 121 can raise (e.g., timeout, DNS/connectivity issue, SSL error, upstream unavailability) before returning a JSON payload, and in that case, execution never reaches:
if not verification.get('success'):
return _error(403, 'reCAPTCHA verification failed', headers)
on line 123, and the Lambda can fail with an unhandled 5xx instead of returning the expected JSON/CORS error shape.
let's add a defensive layer round reCAPTCHA verification in both handlers by wrapping the verification call (or verify_recaptcha internals) in try/except and return a controlled error response (e.g., 503) when reCAPTCHA is temporarily unavailable? that would keep client behavior predictable and avoids hard failures on transient upstream issues, like if reCAPTCHA is temporarily unreachable.
| autoComplete="email" | ||
| aria-required | ||
| {...register('email', { | ||
| required: |
There was a problem hiding this comment.
shouldn't this come from the centralized error messages?
2 participants
No description provided.