fix(app): call contact form logic directly via server action instead …#1360
fix(app): call contact form logic directly via server action instead …#1360andrewklau merged 4 commits intomainfrom
Conversation
…of HTTP round-trip
The contact form server actions were making HTTP requests from the server
back to itself through the public URL (appUrl/api/contact), which routed
traffic through Cloudflare. This caused silent failures — likely blocked
by Cloudflare Page Shield/WAF — with no useful error information reaching
the client.
- Extract shared submitContact() utility from the API route
- Call submitContact() directly in server actions, bypassing the network
- Return structured { success, error } responses to the client
- Display specific error messages instead of generic "Something went wrong!"
- Keep API route as a thin wrapper for external consumers
There was a problem hiding this comment.
Pull request overview
This PR refactors contact-form submission so server actions call shared contact logic directly (SES + Turnstile verification) instead of making server-to-self HTTP requests through the public /api/contact URL (and Cloudflare), and updates client handling to display structured error messages.
Changes:
- Extracted
submitContact()into a shared utility and reused it from server actions and the API route. - Updated contact form UI flows to handle
{ success, error }results instead of treating non-200 responses as generic network failures. - Simplified the
/api/contactroute into a thin wrapper aroundsubmitContact().
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/app/src/utils/app/contact.ts | New shared SES + Turnstile submit utility used by server actions and API route. |
| apps/app/src/app/api/contact/route.ts | Refactored to call submitContact() instead of duplicating logic. |
| apps/app/src/components/app/Contact/ContactOptions.tsx | Server action now calls submitContact() directly. |
| apps/app/src/components/app/Contact/FormContact.tsx | Client-side form now checks response.success and displays response.error. |
| apps/app/src/app/[locale]/apis/page.tsx | Server action now calls submitContact() directly. |
| apps/app/src/components/app/Apis/ApiActions.tsx | Client-side API inquiry flow now checks response.success and displays response.error. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!result.success) { | ||
| return NextResponse.json( | ||
| { err: 'Captcha token is missing', status: 0 }, | ||
| { err: result.error, status: 0 }, | ||
| { status: 400 }, | ||
| ); |
There was a problem hiding this comment.
This handler now maps every submitContact() failure to HTTP 400. That changes prior behavior (e.g. config errors should be 500, Turnstile upstream failures should be 502) and can mislead external API consumers. Preserve status semantics by returning an HTTP status (or error kind) from submitContact() and using it here.
| const contactDetails = { | ||
| description: description, | ||
| email: email, | ||
| name: name, | ||
| subject, | ||
| }; | ||
| const response = await getContactDetails(contactDetails); | ||
| if (!response) { | ||
| throw new Error('Network response was not ok'); | ||
| if (!response?.success) { | ||
| toast.error(response?.error || 'Something went wrong!'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
submitContact() requires a captcha token, but this form doesn’t send one. With the new direct call, this path will deterministically return Captcha token is missing and never send the email. Either add Turnstile/token collection to this form flow, or route these submissions to a separate server action that doesn’t require captcha.
| } | ||
| toast.success('Thank you!'); | ||
| } catch (err) { | ||
| console.log(err); |
There was a problem hiding this comment.
Use console.error (or remove logging) instead of console.log in error handling so failures are classified correctly in logs/observability tooling.
| console.log(err); | |
| console.error(err); |
| const getContactDetails = async (contactDetails: any) => { | ||
| 'use server'; | ||
|
|
||
| const contactRes = await postRequest('/api/contact', contactDeatils); | ||
| return contactRes; | ||
| try { | ||
| return await submitContact(contactDetails); | ||
| } catch (error) { | ||
| console.error('Contact form submission error:', error); | ||
| return { error: 'Failed to submit contact form', success: false }; | ||
| } |
There was a problem hiding this comment.
This server action accepts contactDetails: any even though ContactFormData/ContactResult types exist in @/utils/app/contact. Typing this parameter/return will prevent accidentally omitting required fields (like token) and makes the server action contract clearer.
| const getContactDetails = async (contactDetails: any) => { | ||
| 'use server'; | ||
|
|
||
| const contactRes = await postRequest('/api/contact', contactDeatils); | ||
| return contactRes; | ||
| try { | ||
| return await submitContact(contactDetails); | ||
| } catch (error) { | ||
| console.error('Contact form submission error:', error); | ||
| return { error: 'Failed to submit contact form', success: false }; | ||
| } |
There was a problem hiding this comment.
This server action accepts contactDetails: any even though ContactFormData/ContactResult are defined in @/utils/app/contact. Typing this will help catch missing required fields (notably the captcha token) at compile time.
apps/app/src/utils/app/contact.ts
Outdated
| import { SendEmailCommand, SESClient } from '@aws-sdk/client-ses'; | ||
| import type { TurnstileServerValidationResponse } from '@marsidev/react-turnstile'; | ||
|
|
||
| const sesClient = new SESClient({ | ||
| region: process.env.AWS_REGION, | ||
| }); |
There was a problem hiding this comment.
submitContact uses AWS SES + server env vars, but this module isn’t marked server-only. Add a server-only boundary (e.g. a top-level 'use server'; directive) so it can’t be accidentally imported into client components and bundled with server dependencies.
apps/app/src/utils/app/contact.ts
Outdated
| const sesClient = new SESClient({ | ||
| region: process.env.AWS_REGION, | ||
| }); |
There was a problem hiding this comment.
SESClient is constructed with region: process.env.AWS_REGION but AWS_REGION isn’t validated. If it’s missing/misconfigured, the send will fail at runtime and you’ll lose the structured error flow. Include AWS_REGION in the configuration checks and/or lazily create the SES client after validating config so failures return a controlled { success:false, error }.
apps/app/src/utils/app/contact.ts
Outdated
| export interface ContactResult { | ||
| data?: any; |
There was a problem hiding this comment.
ContactResult.data is typed as any, which makes callers harder to type safely. Consider defining a concrete payload shape (e.g. { message: string }) and using that in ContactResult so server actions/UI can rely on stable typing.
| export interface ContactResult { | |
| data?: any; | |
| export interface ContactResultData { | |
| message: string; | |
| } | |
| export interface ContactResult { | |
| data?: ContactResultData; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/app/src/utils/app/contact.ts
Outdated
| const verificationResponse = await fetch(TURNSTILE_VERIFY_URL, { | ||
| body: formData.toString(), | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| }, | ||
| method: 'POST', | ||
| }); |
There was a problem hiding this comment.
fetch(TURNSTILE_VERIFY_URL, …) can throw (network/DNS/timeout) and verificationResponse.json() can throw on invalid/empty bodies. Right now those exceptions will bypass the structured ContactResult and surface as a generic 500 from the caller. Wrap the Turnstile request+JSON parsing in a try/catch and return a consistent { success:false, error, statusCode } result on failures.
apps/app/src/utils/app/contact.ts
Outdated
| const verificationResponse = await fetch(TURNSTILE_VERIFY_URL, { | ||
| body: formData.toString(), | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| }, | ||
| method: 'POST', | ||
| }); | ||
|
|
There was a problem hiding this comment.
Consider adding an explicit timeout/abort for the Turnstile verification fetch. Without a timeout, this server action can hang waiting on the external service, tying up server resources and making the form appear stuck to users.
| const verificationResponse = await fetch(TURNSTILE_VERIFY_URL, { | |
| body: formData.toString(), | |
| headers: { | |
| 'Content-Type': 'application/x-www-form-urlencoded', | |
| }, | |
| method: 'POST', | |
| }); | |
| const turnstileTimeoutMs = 10_000; | |
| const controller = new AbortController(); | |
| const timeoutId = setTimeout(() => { | |
| controller.abort(); | |
| }, turnstileTimeoutMs); | |
| let verificationResponse: Response; | |
| try { | |
| verificationResponse = await fetch(TURNSTILE_VERIFY_URL, { | |
| body: formData.toString(), | |
| headers: { | |
| 'Content-Type': 'application/x-www-form-urlencoded', | |
| }, | |
| method: 'POST', | |
| signal: controller.signal, | |
| }); | |
| } catch (error) { | |
| if ((error as any)?.name === 'AbortError') { | |
| console.error('Turnstile verification request timed out'); | |
| } else { | |
| console.error('Turnstile verification request failed:', error); | |
| } | |
| return { | |
| error: 'Captcha verification service unavailable', | |
| statusCode: 502, | |
| success: false, | |
| }; | |
| } finally { | |
| clearTimeout(timeoutId); | |
| } |
| Data: ` | ||
| <h2>New Contact Form Submission</h2> | ||
| <p><strong>From:</strong> ${name} (${email})</p> | ||
| <p><strong>Subject:</strong> ${subject}</p> | ||
| <p><strong>Message:</strong></p> | ||
| <p>${description}</p> | ||
| `, |
There was a problem hiding this comment.
User-controlled fields (name, email, subject, description) are interpolated directly into the HTML email body. This allows HTML injection/phishing content in the email. Escape/sanitize these values before embedding in HTML (or send only the text version) to avoid rendering attacker-supplied markup.
apps/app/src/utils/app/contact.ts
Outdated
| const emailResponse = await client.send(command); | ||
| console.log('Email sent successfully:', emailResponse); | ||
|
|
||
| return { | ||
| data: { message: 'Message sent successfully' }, | ||
| statusCode: 200, | ||
| success: true, | ||
| }; |
There was a problem hiding this comment.
client.send(command) can throw (AWS credentials/permissions, SES throttling, invalid params). If it throws, callers won’t get the structured { success, error } response this PR is aiming for. Catch SES send errors and map them to a stable ContactResult (e.g., 502/500 with a user-safe message).
| const emailResponse = await client.send(command); | |
| console.log('Email sent successfully:', emailResponse); | |
| return { | |
| data: { message: 'Message sent successfully' }, | |
| statusCode: 200, | |
| success: true, | |
| }; | |
| try { | |
| const emailResponse = await client.send(command); | |
| console.log('Email sent successfully:', emailResponse); | |
| return { | |
| data: { message: 'Message sent successfully' }, | |
| statusCode: 200, | |
| success: true, | |
| }; | |
| } catch (error) { | |
| console.error('Error sending email via SES:', error); | |
| return { | |
| error: 'Failed to send message. Please try again later.', | |
| statusCode: 502, | |
| success: false, | |
| }; | |
| } |
| ref={turnstileRef} | ||
| siteKey={siteKey as string} | ||
| /> |
There was a problem hiding this comment.
useConfig() can return siteKey as undefined (it’s sourced from NEXT_PUBLIC_TURNSTILE_SITE_KEY). Casting with as string hides this and can cause Turnstile to fail to render or submit at runtime, making the form unusable. Guard against a missing siteKey (disable submit / show configuration error) instead of casting.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/app/src/utils/app/contact.ts
Outdated
| const { description, email, name, subject, token } = body; | ||
|
|
There was a problem hiding this comment.
submitContact destructures body immediately. If the API route receives a non-object JSON value (e.g. null, string, number), this will throw and fall into the generic 500 handler instead of returning a 400. Add a defensive check that body is a non-null object (and ideally that fields are strings) before destructuring, and return a structured 400 error when invalid.
| const { description, email, name, subject, token } = body; | |
| if (!body || typeof body !== 'object') { | |
| return { | |
| error: 'Invalid request body', | |
| statusCode: 400, | |
| success: false, | |
| }; | |
| } | |
| const { description, email, name, subject, token } = body as ContactFormData; | |
| if ( | |
| typeof name !== 'string' || | |
| typeof email !== 'string' || | |
| typeof subject !== 'string' || | |
| typeof description !== 'string' || | |
| typeof token !== 'string' | |
| ) { | |
| return { | |
| error: 'Invalid field types', | |
| statusCode: 400, | |
| success: false, | |
| }; | |
| } |
| const submitForm = async (event: any) => { | ||
| event.preventDefault(); | ||
|
|
||
| if (!siteKey || captchaStatus !== 'solved' || !token) { |
There was a problem hiding this comment.
The !siteKey branch sets captchaStatus to 'error', but when siteKey is missing the UI already shows “Captcha is currently unavailable.” This can also trigger the “Please verify the captcha” error state even though verification is impossible. Consider handling !siteKey separately (e.g. show a dedicated toast/message and avoid setting the verify-captcha error state).
| if (!siteKey || captchaStatus !== 'solved' || !token) { | |
| if (!siteKey) { | |
| toast.error('Captcha is currently unavailable.'); | |
| return; | |
| } | |
| if (captchaStatus !== 'solved' || !token) { |
| {captchaStatus === 'error' && ( | ||
| <span className="text-red-500 text-sm p-6"> | ||
| * Please verify the captcha | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
The “Please verify the captcha” message is shown whenever captchaStatus === 'error', even when siteKey is missing and the captcha widget is not rendered. This produces a confusing error state. Only show this message when the widget is available, or render a different message for the !siteKey case.
| try { | ||
| return await submitContact(contactDetails); | ||
| } catch (error) { | ||
| console.error('Contact form submission error:', error); | ||
| return { | ||
| error: 'Failed to submit contact form', | ||
| statusCode: 500, | ||
| success: false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
This server action wrapper duplicates the same try/catch + fallback ContactResult pattern used elsewhere (e.g. ApisPage). Consider extracting a shared server action helper (or a small wrapper around submitContact) to keep error handling consistent and avoid diverging behavior over time.
| try { | ||
| return await submitContact(contactDetails); | ||
| } catch (error) { | ||
| console.error('Contact form submission error:', error); | ||
| return { | ||
| error: 'Failed to submit contact form', | ||
| statusCode: 500, | ||
| success: false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
This server action wrapper is effectively identical to the one in ContactOptions. Consider consolidating into a shared helper (or exporting a single getContactDetails server action) to avoid duplicated error handling logic across pages.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const safeName = escapeHtml(name); | ||
| const safeEmail = escapeHtml(email); | ||
| const safeSubject = escapeHtml(subject); | ||
| const safeDescription = escapeHtml(description); | ||
|
|
||
| const params = { | ||
| Destination: { | ||
| ToAddresses: [TO_EMAIL], | ||
| }, | ||
| Message: { | ||
| Body: { | ||
| Html: { | ||
| Data: ` | ||
| <h2>New Contact Form Submission</h2> | ||
| <p><strong>From:</strong> ${safeName} (${safeEmail})</p> | ||
| <p><strong>Subject:</strong> ${safeSubject}</p> | ||
| <p><strong>Message:</strong></p> | ||
| <p>${safeDescription}</p> | ||
| `, | ||
| }, | ||
| Text: { | ||
| Data: `From: ${name} (${email})\nSubject: ${subject}\n\n${description}`, | ||
| }, | ||
| }, | ||
| Subject: { | ||
| Data: subject, | ||
| }, |
There was a problem hiding this comment.
safeSubject is computed but the SES Subject.Data still uses the raw subject, and the plain-text body uses raw name/email/subject/description while the HTML body is escaped. This can lead to inconsistent output and leaves room for header/control-character injection (e.g., CRLF in subject). Consider sanitizing subject for use in headers (strip \r/\n) and using the sanitized/escaped values consistently across both HTML and text parts.
| if (!result.success) { | ||
| return NextResponse.json( | ||
| { err: 'Server configuration error', status: 0 }, | ||
| { status: 500 }, | ||
| { err: result.error, status: 0 }, | ||
| { status: result.statusCode }, | ||
| ); | ||
| } | ||
|
|
||
| if (!token) { | ||
| return NextResponse.json( | ||
| { err: 'Captcha token is missing', status: 0 }, | ||
| { status: 400 }, | ||
| ); | ||
| } | ||
|
|
||
| const formData = new URLSearchParams(); | ||
| formData.append('secret', TURNSTILE_SECRET_KEY); | ||
| formData.append('response', token); | ||
|
|
||
| const verificationResponse = await fetch(TURNSTILE_VERIFY_URL, { | ||
| body: formData.toString(), | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| }, | ||
| method: 'POST', | ||
| }); | ||
|
|
||
| if (!verificationResponse.ok) { | ||
| console.error( | ||
| 'Turnstile verification request failed:', | ||
| verificationResponse.status, | ||
| ); | ||
| return NextResponse.json( | ||
| { err: 'Captcha verification service unavailable', status: 0 }, | ||
| { status: 502 }, | ||
| ); | ||
| } | ||
|
|
||
| const data = | ||
| (await verificationResponse.json()) as TurnstileServerValidationResponse; | ||
|
|
||
| if (!data.success) { | ||
| console.log('Captcha verification failed:', data); | ||
| return NextResponse.json( | ||
| { details: data, err: 'Captcha verification failed', status: 0 }, | ||
| { status: 400 }, | ||
| ); | ||
| } | ||
|
|
||
| const params = { | ||
| Destination: { | ||
| ToAddresses: [TO_EMAIL], | ||
| }, | ||
| Message: { | ||
| Body: { | ||
| Html: { | ||
| Data: ` | ||
| <h2>New Contact Form Submission</h2> | ||
| <p><strong>From:</strong> ${name} (${email})</p> | ||
| <p><strong>Subject:</strong> ${subject}</p> | ||
| <p><strong>Message:</strong></p> | ||
| <p>${description}</p> | ||
| `, | ||
| }, | ||
| Text: { | ||
| Data: `From: ${name} (${email})\nSubject: ${subject}\n\n${description}`, | ||
| }, | ||
| }, | ||
| Subject: { | ||
| Data: subject, | ||
| }, | ||
| }, | ||
| Source: FROM_EMAIL, | ||
| }; | ||
|
|
||
| const command = new SendEmailCommand(params); | ||
| const emailResponse = await sesClient.send(command); | ||
| console.log('Email sent successfully:', emailResponse); | ||
|
|
||
| return NextResponse.json( | ||
| { message: 'Message sent successfully', status: 1 }, | ||
| { status: 200 }, | ||
| { message: result.data?.message, status: 1 }, | ||
| { status: result.statusCode }, | ||
| ); |
There was a problem hiding this comment.
The API response now sets message from result.data?.message, which can be undefined if submitContact ever returns success: true without data. For external consumers, it’s safer to always return a string message (e.g., fallback to a default) and ensure err is always a string in error cases to keep the response contract stable.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| }, | ||
| Subject: { | ||
| Data: subject, | ||
| }, |
There was a problem hiding this comment.
subject is used directly in SES Message.Subject.Data (and is also interpolated into the text body). If subject contains CR/LF or exceeds SES subject limits, SES can reject the request. Consider normalizing (trim + strip \r/\n/control chars) and enforcing a max length before passing it to SES.
| try { | ||
| const body = (await request.json()) as ContactFormData; | ||
| const result = await submitContact(body); | ||
|
|
||
| const { description, email, name, subject, token } = body; | ||
|
|
||
| if (!name || !email || !subject || !description) { | ||
| return NextResponse.json( | ||
| { err: 'Missing required fields', status: 0 }, | ||
| { status: 400 }, | ||
| ); | ||
| } | ||
|
|
||
| if (!TO_EMAIL || !FROM_EMAIL || !TURNSTILE_SECRET_KEY) { | ||
| if (!result.success) { |
There was a problem hiding this comment.
If request.json() throws (invalid/malformed JSON), this handler currently returns a 500. Since that’s a client error, consider catching JSON parse errors separately and returning a 400 with an "Invalid JSON"-style message so consumers can distinguish bad input from server failures.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Data: `From: ${name} (${email})\nSubject: ${subject}\n\n${description}`, | ||
| }, | ||
| }, | ||
| Subject: { | ||
| Data: subject, |
There was a problem hiding this comment.
The HTML body uses the escaped values (safeName/safeEmail/safeSubject/safeDescription), but the text body and Subject header still use the raw user inputs. This inconsistency makes it easy to miss sanitization assumptions and can allow control characters/newlines into the email subject/body; consider using the sanitized variants or explicitly stripping CR/LF/control characters for name/email/subject before building the SES params.
| Data: `From: ${name} (${email})\nSubject: ${subject}\n\n${description}`, | |
| }, | |
| }, | |
| Subject: { | |
| Data: subject, | |
| Data: `From: ${safeName} (${safeEmail})\nSubject: ${safeSubject}\n\n${safeDescription}`, | |
| }, | |
| }, | |
| Subject: { | |
| Data: safeSubject, |
| submitContact, | ||
| } from '@/utils/app/contact'; | ||
|
|
||
| export async function getContactDetails( |
There was a problem hiding this comment.
The server action is named getContactDetails, but it actually submits the contact form. Renaming it to something like submitContact/submitContactForm would make call sites clearer and avoid confusing this with a read operation.
| export async function getContactDetails( | |
| export async function submitContactForm( |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| appearance: 'always', | ||
| refreshExpired: 'auto', | ||
| size: 'normal', | ||
| theme: theme as any, |
There was a problem hiding this comment.
theme from next-themes can be 'system', and casting to any may pass an unsupported value to Turnstile, causing incorrect rendering or runtime issues. Map to an explicit 'light' | 'dark' value (e.g., via resolvedTheme) instead of theme as any.
| theme: theme as any, | |
| theme: theme === 'dark' ? 'dark' : 'light', |
| Data: `From: ${name} (${email})\nSubject: ${subject}\n\n${description}`, | ||
| }, | ||
| }, | ||
| Subject: { | ||
| Data: subject, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
subject (and other user-controlled fields) are used directly in SES Subject.Data and the text body. To avoid header/control-character injection and reduce SES rejections, strip \r/\n (and possibly enforce length limits) before using these values in Subject/email content.
The contact form server actions were making HTTP requests from the server back to itself through the public URL (appUrl/api/contact), which routed traffic through Cloudflare. This caused silent failures — likely blocked by Cloudflare Page Shield/WAF — with no useful error information reaching the client.