fix: improve newsletter subscription error handling for service outages #4798
Conversation
Khushboo1324
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg,
princerajpoot20 and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
📝 WalkthroughWalkthroughAdds a new FormStatus member Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User (Browser)
participant C as NewsletterSubscribe Component
participant S as Newsletter API (Server)
U->>C: submit form (name, email)
C->>S: fetch POST /subscribe
alt 200 OK
S-->>C: 200 + success body
C->>C: set FormStatus.SUCCESS
Note right of C: render success message
else 503 Service Unavailable
S-->>C: 503
C->>C: set FormStatus.SERVICE_UNAVAILABLE
Note right of C: render temporary unavailability message
else other error status
S-->>C: other status
C->>C: set FormStatus.ERROR
Note right of C: render generic error message
end
opt Network failure / fetch reject
C-->>C: catch -> set FormStatus.SERVICE_UNAVAILABLE
Note right of C: render temporary unavailability message
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR enhances error handling in the NewsletterSubscribe component to provide better user feedback during service outages and network failures. It introduces a new SERVICE_UNAVAILABLE form status and updates the fetch error handling logic to distinguish between different failure scenarios.
Key changes:
- Added
SERVICE_UNAVAILABLEstatus to the FormStatus enum and corresponding UI rendering - Implemented explicit handling for 401 responses and network failures by mapping them to the service unavailable state
- Improved JSON parsing safety with try-catch to handle malformed responses
- Code formatting improvements (single to double quotes, consistent enum usage)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
components/NewsletterSubscribe.tsx
Outdated
| .then(async (res) => { | ||
| if (res.status === 200) { | ||
| setFormStatus(FormStatus.SUCCESS); | ||
| } else if (res.status === 401) { |
There was a problem hiding this comment.
A 401 Unauthorized status typically indicates authentication/authorization issues, not service unavailability. The PR description mentions handling service outages, but 401 is not the appropriate status code for this. Consider using 503 Service Unavailable instead, or handle 401 separately as an authentication error.
| } else if (res.status === 401) { | |
| } else if (res.status === 503) { |
| Newsletter service temporarily unavailable | ||
| </Heading> | ||
| <Paragraph className="mb-8" textColor={paragraphTextColor}> | ||
| Please try again later. |
There was a problem hiding this comment.
The new SERVICE_UNAVAILABLE state does not use internationalization (i18n) for its messages, unlike the SUCCESS and ERROR states which use the translation hook. The hardcoded strings "Newsletter service temporarily unavailable" and "Please try again later." should be added to the translation files and retrieved using the t() function for consistency with the rest of the component.
| Newsletter service temporarily unavailable | |
| </Heading> | |
| <Paragraph className="mb-8" textColor={paragraphTextColor}> | |
| Please try again later. | |
| {ready | |
| ? t('serviceUnavailableTitle') | |
| : 'Newsletter service temporarily unavailable'} | |
| </Heading> | |
| <Paragraph className="mb-8" textColor={paragraphTextColor}> | |
| {ready ? t('serviceUnavailableSubtitle') : 'Please try again later.'} |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4798--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/NewsletterSubscribe.tsx (2)
140-159: Fix quote style to pass linting.Multiple lines have double quotes that should be single quotes per the project's Prettier configuration.
🔎 Proposed fix
if (status === FormStatus.ERROR) { return ( - <div className={className} data-testid="NewsletterSubscribe-main"> + <div className={className} data-testid='NewsletterSubscribe-main'> <Heading level={HeadingLevel.h3} textColor={headTextColor} typeStyle={HeadingTypeStyle.lg} - className="mb-4" + className='mb-4' > {ready ? t('errorTitle') : 'Something went wrong'} </Heading> - <Paragraph className="mb-8" textColor={paragraphTextColor}> + <Paragraph className='mb-8' textColor={paragraphTextColor}> {ready ? t('errorSubtitle') : errorSubtitle}{' '} <TextLink - href="https://github.com/asyncapi/website/issues/new?template=bug.md" - target="_blank" + href='https://github.com/asyncapi/website/issues/new?template=bug.md' + target='_blank' > {ready ? t('errorLinkText') : 'here'} </TextLink>
163-209: Fix quote style and review empty href.Multiple lines have double quotes that should be single quotes. Additionally, line 204 has an empty
hrefattribute on a submit button, which seems unnecessary.🔎 Proposed fix for quote style
return ( - <div className={className} data-testid="NewsletterSubscribe-main"> + <div className={className} data-testid='NewsletterSubscribe-main'> <Heading level={HeadingLevel.h3} textColor={headTextColor} typeStyle={HeadingTypeStyle.lg} - className="mb-4" + className='mb-4' > {ready ? t('title') : title} </Heading> - <Paragraph className="mb-8" textColor={paragraphTextColor}> + <Paragraph className='mb-8' textColor={paragraphTextColor}> {ready ? t('subtitle') : subtitle} </Paragraph> {status === FormStatus.LOADING ? ( <Loader - loaderText={'Waiting for response...'} + loaderText='Waiting for response...' loaderIcon={<IconCircularLoader dark />} dark={dark} /> ) : ( <form - className="flex flex-col gap-4 md:flex-row" + className='flex flex-col gap-4 md:flex-row' onSubmit={handleSubmit} > <InputBox inputType={InputTypes.TEXT} - inputName="name" + inputName='name' placeholder={ready ? t('nameInput') : 'Your name'} inputValue={name} setInput={setName} /> <InputBox inputType={InputTypes.EMAIL} - inputName="email" + inputName='email' placeholder={ready ? t('emailInput') : 'Your email'} inputValue={email} setInput={setEmail} /> <Button type={ButtonType.SUBMIT} text={ready ? t('subscribeBtn') : 'Subscribe'} - className="mt-2 w-full md:mr-2 md:mt-0 md:flex-1" - href="" + className='mt-2 w-full md:mr-2 md:mt-0 md:flex-1' + href='' />💡 Optional: Consider removing the empty href
The
Buttoncomponent hastype={ButtonType.SUBMIT}, which makes it a submit button. The emptyhref=''attribute appears unnecessary. Consider checking if the Button component requires an href prop or if it can be omitted for submit buttons:<Button type={ButtonType.SUBMIT} text={ready ? t('subscribeBtn') : 'Subscribe'} className='mt-2 w-full md:mr-2 md:mt-0 md:flex-1' - href='' />
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/NewsletterSubscribe.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/NewsletterSubscribe.tsx (7)
components/typography/Heading.tsx (1)
Heading(30-85)components/typography/Paragraph.tsx (1)
Paragraph(28-56)components/typography/TextLink.tsx (1)
TextLink(21-42)components/Loader.tsx (1)
Loader(28-41)components/icons/CircularLoader.tsx (1)
IconCircularLoader(13-20)components/InputBox.tsx (1)
InputBox(8-21)components/buttons/Button.tsx (1)
Button(53-106)
🪛 GitHub Actions: PR testing - if Node project
components/NewsletterSubscribe.tsx
[error] 50-50: prettier/prettier: Delete , and trailing comma.
[error] 74-74: prettier/prettier: Delete , and trailing comma.
[error] 81-81: prettier/prettier: Delete , and trailing comma.
[error] 82-82: prettier/prettier: Delete , and trailing comma.
[error] 106-106: prettier/prettier: Replace "NewsletterSubscribe-main" with 'NewsletterSubscribe-main' and jsx-quotes.
[error] 107-107: jsx-quotes: Unexpected usage of doublequote.
[error] 111-111: prettier/prettier: Unexpected usage of doublequote.
[error] 115-115: prettier/prettier: Replace "mb-8" with 'mb-8'.
[error] 123-123: prettier/prettier: Replace "NewsletterSubscribe-main" with 'NewsletterSubscribe-main' and jsx-quotes.
[error] 124-124: jsx-quotes: Unexpected usage of doublequote.
[error] 128-128: prettier/prettier: Unexpected usage of doublequote.
[error] 132-132: prettier/prettier: Replace "mb-8" with 'mb-8' and jsx-quotes.
[error] 140-140: prettier/prettier: Replace "NewsletterSubscribe-main" with 'NewsletterSubscribe-main' and jsx-quotes.
[error] 145-145: prettier/prettier: Unexpected usage of doublequote.
[error] 149-149: prettier/prettier: Replace "mb-8" with 'mb-8'.
[error] 151-151: prettier/prettier: Unexpected usage of doublequote.
[error] 163-163: prettier/prettier: Replace "NewsletterSubscribe-main" with 'NewsletterSubscribe-main' and jsx-quotes.
[error] 164-164: jsx-quotes: Unexpected usage of doublequote.
[error] 168-168: prettier/prettier: Unexpected usage of doublequote.
[error] 172-172: prettier/prettier: Replace "mb-8" with 'mb-8'.
[error] 176-176: prettier/prettier: Unexpected usage of doublequote.
[error] 182-182: prettier/prettier: Replace "flex flex-col gap-4 md:flex-row" with 'flex flex-col gap-4 md:flex-row' and onSubmit prop formatting.
[error] 183-183: jsx-quotes: Unexpected usage of doublequote.
[error] 188-188: prettier/prettier: Replace "name" with 'name' and jsx-quotes.
[error] 195-195: prettier/prettier: Replace "email" with 'email' and jsx-quotes.
[error] 203-203: prettier/prettier: Replace "mt-2 w-full md:mr-2 md:mt-0 md:flex-1" with 'mt-2 w-full md:mr-2 md:mt-0 md:flex-1' and jsx-quotes.
[error] 204-204: prettier/prettier: Empty string should be '' not "".
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lighthouse CI
🔇 Additional comments (4)
components/NewsletterSubscribe.tsx (4)
20-21: LGTM! Clean enum extension.The new
SERVICE_UNAVAILABLEstatus value is well-named and appropriately added to theFormStatusenum.
93-97: LGTM! Good defensive programming.The try-catch around
res.json()with a null fallback gracefully handles malformed JSON responses and prevents unhandled promise rejections.
99-101: LGTM! Appropriate network failure handling.Mapping network failures to
SERVICE_UNAVAILABLEis appropriate and provides a better user experience than a generic error message.
84-91: The 401 → SERVICE_UNAVAILABLE mapping is semantically incorrect.HTTP 401 (Unauthorized) indicates authentication/authorization failure—typically a configuration issue like an invalid API key—not a temporary service outage. HTTP 503 is the standard status for "Service Unavailable." The user-facing message "Please try again later" implies a temporary condition, but 401 represents a persistent configuration problem.
While the PR objectives mention handling 401 explicitly, this mapping misleads users into thinking a temporary issue will resolve itself when the actual problem is likely a misconfigured API key.
Recommend either:
- Mapping 401 to ERROR (authentication failure) instead
- Only mapping 503 responses to SERVICE_UNAVAILABLE
- Adding clear documentation explaining why 401 is treated as a service outage
| if (status === FormStatus.SERVICE_UNAVAILABLE) { | ||
| return ( | ||
| <div className={className} data-testid="NewsletterSubscribe-main"> | ||
| <Heading | ||
| level={HeadingLevel.h3} | ||
| textColor={headTextColor} | ||
| typeStyle={HeadingTypeStyle.lg} | ||
| className="mb-4" | ||
| > | ||
| Newsletter service temporarily unavailable | ||
| </Heading> | ||
| <Paragraph className="mb-8" textColor={paragraphTextColor}> | ||
| Please try again later. | ||
| </Paragraph> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Fix quote style and consider internationalization.
Multiple lines have double quotes that should be single quotes. Additionally, unlike other states, this message is not internationalized using the ready check and t() function.
🔎 Proposed fix for quote style
if (status === FormStatus.SERVICE_UNAVAILABLE) {
return (
- <div className={className} data-testid="NewsletterSubscribe-main">
+ <div className={className} data-testid='NewsletterSubscribe-main'>
<Heading
level={HeadingLevel.h3}
textColor={headTextColor}
typeStyle={HeadingTypeStyle.lg}
- className="mb-4"
+ className='mb-4'
>
Newsletter service temporarily unavailable
</Heading>
- <Paragraph className="mb-8" textColor={paragraphTextColor}>
+ <Paragraph className='mb-8' textColor={paragraphTextColor}>
Please try again later.
</Paragraph>
</div>💡 Optional: Add i18n support
To maintain consistency with other states, consider adding translation keys for the SERVICE_UNAVAILABLE messages in your translation files and using them with the ready check:
>
- Newsletter service temporarily unavailable
+ {ready ? t('serviceUnavailableTitle') : 'Newsletter service temporarily unavailable'}
</Heading>
- <Paragraph className='mb-8' textColor={paragraphTextColor}>
- Please try again later.
+ <Paragraph className='mb-8' textColor={paragraphTextColor}>
+ {ready ? t('serviceUnavailableSubtitle') : 'Please try again later.'}
</Paragraph>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (status === FormStatus.SERVICE_UNAVAILABLE) { | |
| return ( | |
| <div className={className} data-testid="NewsletterSubscribe-main"> | |
| <Heading | |
| level={HeadingLevel.h3} | |
| textColor={headTextColor} | |
| typeStyle={HeadingTypeStyle.lg} | |
| className="mb-4" | |
| > | |
| Newsletter service temporarily unavailable | |
| </Heading> | |
| <Paragraph className="mb-8" textColor={paragraphTextColor}> | |
| Please try again later. | |
| </Paragraph> | |
| </div> | |
| ); | |
| } | |
| if (status === FormStatus.SERVICE_UNAVAILABLE) { | |
| return ( | |
| <div className={className} data-testid='NewsletterSubscribe-main'> | |
| <Heading | |
| level={HeadingLevel.h3} | |
| textColor={headTextColor} | |
| typeStyle={HeadingTypeStyle.lg} | |
| className='mb-4' | |
| > | |
| Newsletter service temporarily unavailable | |
| </Heading> | |
| <Paragraph className='mb-8' textColor={paragraphTextColor}> | |
| Please try again later. | |
| </Paragraph> | |
| </div> | |
| ); | |
| } |
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 123-123: prettier/prettier: Replace "NewsletterSubscribe-main" with 'NewsletterSubscribe-main' and jsx-quotes.
[error] 124-124: jsx-quotes: Unexpected usage of doublequote.
[error] 128-128: prettier/prettier: Unexpected usage of doublequote.
[error] 132-132: prettier/prettier: Replace "mb-8" with 'mb-8' and jsx-quotes.
🤖 Prompt for AI Agents
In components/NewsletterSubscribe.tsx around lines 121-137, the
SERVICE_UNAVAILABLE block uses double quotes and hard-coded English strings;
change string literals to single quotes to match project style and replace the
hard-coded Heading/Paragraph text with i18n lookups using the existing ready
check and t() function (add/consume translation keys like
newsletter.serviceUnavailable.title and newsletter.serviceUnavailable.message),
ensuring you only render translated text when ready to avoid SSR issues.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .catch(() => { | ||
| setFormStatus(FormStatus.SERVICE_UNAVAILABLE); | ||
| }); |
There was a problem hiding this comment.
The catch block will catch network errors and fetch failures, but will also catch any errors thrown in the preceding then block. If the JSON parsing fails or if there's an error in setFormStatus, this will incorrectly set the status to SERVICE_UNAVAILABLE. Consider handling network errors separately or ensuring the catch only handles the specific error types it's intended for.
components/NewsletterSubscribe.tsx
Outdated
| return await res.json(); | ||
| } catch { | ||
| return null; |
There was a problem hiding this comment.
The parsed JSON response is returned but never used or stored anywhere. This return value from the promise chain has no effect since there's no variable assignment or further then() handler. Consider either removing the return statement and JSON parsing if the response body isn't needed, or storing it for potential future use or logging.
| return await res.json(); | |
| } catch { | |
| return null; | |
| await res.json(); | |
| } catch { | |
| // Response body is ignored |
Khushboo1324
changed the title
|
Hi maintainers , |
|
@Khushboo1324 Please have a look at the failed workflow. It is failing because of your changes. You can analyze the run to understand it better. let me know you still need more help |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4798 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 798 796 -2
Branches 146 146
=========================================
- Hits 798 796 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @components/NewsletterSubscribe.tsx:
- Around line 77-78: The branch that treats HTTP 401 as service unavailability
is incorrect: change the check on res.status from 401 to 503 and
setFormStatus(FormStatus.SERVICE_UNAVAILABLE) when res.status === 503;
additionally, if you need to handle authentication issues, add a separate branch
for res.status === 401 (e.g., setFormStatus(FormStatus.AUTH_ERROR) or another
appropriate enum member) so authentication and service outages are mapped to
distinct FormStatus values; update any conditional logic in
NewsletterSubscribe.tsx that references res.status, setFormStatus, and the
FormStatus enum accordingly.
- Around line 83-91: The catch currently catches JSON parse errors from
res.json() and treats them as network failures; in the NewsletterSubscribe
component change the fetch then-chain so you call
setFormStatus(FormStatus.SERVICE_UNAVAILABLE) (or handle response status) before
attempting res.json(), or remove the res.json() entirely if the body isn’t used;
ensure res.json() parsing happens in a separate try/catch so JSON errors are
handled separately and do not trigger the network-error handler tied to
setFormStatus(FormStatus.SERVICE_UNAVAILABLE).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/NewsletterSubscribe.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-03-16T15:55:44.257Z
Learnt from: JeelRajodiya
Repo: asyncapi/website PR: 3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Applied to files:
components/NewsletterSubscribe.tsx
📚 Learning: 2024-12-31T09:25:52.573Z
Learnt from: JeelRajodiya
Repo: asyncapi/website PR: 3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Applied to files:
components/NewsletterSubscribe.tsx
📚 Learning: 2025-12-29T14:21:28.216Z
Learnt from: sammy200-ui
Repo: asyncapi/website PR: 4804
File: components/navigation/Filter.tsx:32-41
Timestamp: 2025-12-29T14:21:28.216Z
Learning: In the asyncapi/website repository, when you intentionally omit dependencies in React hooks (e.g., useEffect, useMemo), add an eslint-disable comment with an explanatory note above the line to justify the design choice. For example: // eslint-disable-next-line react-hooks/exhaustive-deps: intentionally omitting dependencies to avoid unnecessary re-runs. This helps reviewers understand the rationale and keeps lint guidance informative.
Applied to files:
components/NewsletterSubscribe.tsx
🧬 Code graph analysis (1)
components/NewsletterSubscribe.tsx (2)
components/typography/Heading.tsx (1)
Heading(30-85)components/typography/Paragraph.tsx (1)
Paragraph(28-56)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (2)
components/NewsletterSubscribe.tsx (2)
20-21: LGTM: SERVICE_UNAVAILABLE status added correctly.The new enum member follows the existing pattern and provides a distinct state for service outages.
144-144: LGTM: Consistent enum-based status check.Switching from a string literal to
FormStatus.LOADINGimproves type safety and consistency with the rest of the refactored code.
components/NewsletterSubscribe.tsx
Outdated
| } else if (res.status === 401) { | ||
| setFormStatus(FormStatus.SERVICE_UNAVAILABLE); |
There was a problem hiding this comment.
Use HTTP 503 for service unavailability, not 401.
HTTP 401 indicates authentication/authorization errors, not service outages. For service unavailability, the standard status code is 503. Mapping 401 to SERVICE_UNAVAILABLE creates a semantic mismatch and may confuse users with incorrect messaging.
🔎 Proposed fix
- } else if (res.status === 401) {
+ } else if (res.status === 503) {
setFormStatus(FormStatus.SERVICE_UNAVAILABLE);If you need to handle both 503 and authentication errors separately:
if (res.status === 200) {
setFormStatus(FormStatus.SUCCESS);
+ } else if (res.status === 503) {
+ setFormStatus(FormStatus.SERVICE_UNAVAILABLE);
- } else if (res.status === 401) {
- setFormStatus(FormStatus.SERVICE_UNAVAILABLE);
} else {
setFormStatus(FormStatus.ERROR);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (res.status === 401) { | |
| setFormStatus(FormStatus.SERVICE_UNAVAILABLE); | |
| } else if (res.status === 503) { | |
| setFormStatus(FormStatus.SERVICE_UNAVAILABLE); |
🤖 Prompt for AI Agents
In @components/NewsletterSubscribe.tsx around lines 77 - 78, The branch that
treats HTTP 401 as service unavailability is incorrect: change the check on
res.status from 401 to 503 and setFormStatus(FormStatus.SERVICE_UNAVAILABLE)
when res.status === 503; additionally, if you need to handle authentication
issues, add a separate branch for res.status === 401 (e.g.,
setFormStatus(FormStatus.AUTH_ERROR) or another appropriate enum member) so
authentication and service outages are mapped to distinct FormStatus values;
update any conditional logic in NewsletterSubscribe.tsx that references
res.status, setFormStatus, and the FormStatus enum accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/NewsletterSubscribe.tsx (1)
74-85: Remove unnecessaryasynckeyword.The callback is marked
asyncbut contains noawaitexpressions. Since JSON parsing was removed, theasyncmodifier serves no purpose.🔎 Proposed fix
- .then(async (res) => { + .then((res) => { if (res.status === 200) { setFormStatus(FormStatus.SUCCESS); } else if (res.status === 503) { setFormStatus(FormStatus.SERVICE_UNAVAILABLE); } else { setFormStatus(FormStatus.ERROR); } })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/NewsletterSubscribe.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-03-16T15:55:44.257Z
Learnt from: JeelRajodiya
Repo: asyncapi/website PR: 3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Applied to files:
components/NewsletterSubscribe.tsx
📚 Learning: 2024-12-31T09:25:52.573Z
Learnt from: JeelRajodiya
Repo: asyncapi/website PR: 3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Applied to files:
components/NewsletterSubscribe.tsx
📚 Learning: 2025-12-29T14:21:28.216Z
Learnt from: sammy200-ui
Repo: asyncapi/website PR: 4804
File: components/navigation/Filter.tsx:32-41
Timestamp: 2025-12-29T14:21:28.216Z
Learning: In the asyncapi/website repository, when you intentionally omit dependencies in React hooks (e.g., useEffect, useMemo), add an eslint-disable comment with an explanatory note above the line to justify the design choice. For example: // eslint-disable-next-line react-hooks/exhaustive-deps: intentionally omitting dependencies to avoid unnecessary re-runs. This helps reviewers understand the rationale and keeps lint guidance informative.
Applied to files:
components/NewsletterSubscribe.tsx
🧬 Code graph analysis (1)
components/NewsletterSubscribe.tsx (2)
components/typography/Heading.tsx (1)
Heading(30-85)components/typography/Paragraph.tsx (1)
Paragraph(28-56)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (2)
components/NewsletterSubscribe.tsx (2)
16-22: LGTM!The
SERVICE_UNAVAILABLEenum member follows the existing naming convention and is correctly placed within theFormStatusenum.
138-138: LGTM!Using enum-based comparison (
FormStatus.LOADING) instead of string literals improves type safety and consistency with the other status checks.
|
I’ve addressed the lint, semantics and formatting issues causing the CI failure |
3 participants
Summary
This PR improves error handling in the NewsletterSubscribe component by
distinguishing service outages from generic failures.
What Changed
SERVICE_UNAVAILABLEstatus to the form stateRelated Issues
Fixes #4643
Related to #4686
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.