fix: add shutter api env set check #2077
Merged
Conversation
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
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughA guard clause was introduced at the beginning of the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: jaybuidl
PR: kleros/kleros-v2#1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:88-90
Timestamp: 2024-10-09T10:22:41.474Z
Learning: Next.js recommends using the `useEffect` hook to set `isClient` and using `suppressHydrationWarning` as a workaround for handling hydration inconsistencies, especially when dealing with data like `knownArbitrables` that may differ between server-side and client-side rendering. This approach is acceptable in TypeScript/React applications, such as in `web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx`.
📚 Learning: 2024-10-09T10:22:41.474Z
Learnt from: jaybuidl
PR: kleros/kleros-v2#1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:88-90
Timestamp: 2024-10-09T10:22:41.474Z
Learning: Next.js recommends using the `useEffect` hook to set `isClient` and using `suppressHydrationWarning` as a workaround for handling hydration inconsistencies, especially when dealing with data like `knownArbitrables` that may differ between server-side and client-side rendering. This approach is acceptable in TypeScript/React applications, such as in `web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx`.
Applied to files:
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
📚 Learning: 2024-11-21T05:38:11.576Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1755
File: kleros-app/src/lib/atlas/hooks/useSessionStorage.ts:3-12
Timestamp: 2024-11-21T05:38:11.576Z
Learning: In the `useSessionStorage` hook in `kleros-app/src/lib/atlas/hooks/useSessionStorage.ts`, the error handling in the `catch` block covers cases where `window` is undefined or `sessionStorage` throws an exception, so additional checks are unnecessary.
Applied to files:
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
📚 Learning: 2024-10-09T10:18:51.089Z
Learnt from: jaybuidl
PR: kleros/kleros-v2#1582
File: web-devtools/src/app/(main)/ruler/RulingModes.tsx:179-199
Timestamp: 2024-10-09T10:18:51.089Z
Learning: In `web-devtools/src/app/(main)/ruler/RulingModes.tsx`, the `handleUpdate` function already handles errors via `wrapWithToast`, so additional error handling is unnecessary.
Applied to files:
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
📚 Learning: 2024-12-16T17:17:32.359Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1794
File: web/src/hooks/useStarredCases.tsx:13-18
Timestamp: 2024-12-16T17:17:32.359Z
Learning: In `useStarredCases.tsx`, when handling the `starredCases` Map from local storage, direct mutation is acceptable to prevent the overhead of copying, provided it doesn't adversely affect React's render cycle.
Applied to files:
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
📚 Learning: 2024-10-15T16:18:32.543Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1687
File: web/src/context/AtlasProvider.tsx:225-244
Timestamp: 2024-10-15T16:18:32.543Z
Learning: In `web/src/context/AtlasProvider.tsx`, the `atlasUri` variable comes from environment variables and does not change, so it does not need to be included in dependency arrays.
Applied to files:
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
📚 Learning: 2024-12-06T13:04:50.495Z
Learnt from: kemuru
PR: kleros/kleros-v2#1774
File: web/src/components/CasesDisplay/index.tsx:61-61
Timestamp: 2024-12-06T13:04:50.495Z
Learning: In `web/src/components/CasesDisplay/index.tsx`, the variables `numberDisputes` and `numberClosedDisputes` can sometimes be `NaN`, and should default to `0` using logical OR (`||`) to prevent display issues in the `StatsAndFilters` component.
Applied to files:
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
📚 Learning: 2024-10-29T10:14:52.512Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1729
File: web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx:125-127
Timestamp: 2024-10-29T10:14:52.512Z
Learning: In `web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx`, when `isEmailUpdateable` is false, `user?.emailUpdateableAt` is always defined. Therefore, using the non-null assertion `!` with `user?.emailUpdateableAt!` is acceptable because TypeScript may not infer its definiteness.
Applied to files:
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
📚 Learning: 2024-10-29T10:13:04.524Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1729
File: web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx:69-69
Timestamp: 2024-10-29T10:13:04.524Z
Learning: In `web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx`, when the button is disabled, the associated logic won't be reached, and certain code paths may exist for TypeScript purposes.
Applied to files:
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
📚 Learning: 2024-11-07T10:48:16.774Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1739
File: web/src/pages/Home/TopJurors/JurorCard/Coherency.tsx:22-26
Timestamp: 2024-11-07T10:48:16.774Z
Learning: In the `Coherency` component (`web/src/pages/Home/TopJurors/JurorCard/Coherency.tsx`), `totalResolvedVotes` is always greater than or equal to `totalCoherentVotes`. When both are zero, `0/0` results in `NaN`, which is acceptable in this context.
Applied to files:
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: contracts-testing
Comment on lines
88
to
91
| if (!import.meta.env.REACT_APP_SHUTTER_API) { | ||
| console.error("REACT_APP_SHUTTER_API environment variable is not set"); | ||
| throw new Error("Cannot commit vote: REACT_APP_SHUTTER_API environment variable is required but not set"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Env var prefix likely incorrect for Vite; guard will always fail. Also avoid throwing in UI handler.
This app uses import.meta.env (Vite). Client-exposed env vars must be prefixed with VITE_. Checking REACT_APP_SHUTTER_API (CRA-style) will be undefined at runtime, causing this path to always throw and break committing. Prefer VITE_SHUTTER_API, optionally falling back to REACT_APP_SHUTTER_API for backward compatibility. Also, throwing here creates an unhandled rejection UX; return early and surface a user-facing message instead.
Apply:
- if (!import.meta.env.REACT_APP_SHUTTER_API) {
- console.error("REACT_APP_SHUTTER_API environment variable is not set");
- throw new Error("Cannot commit vote: REACT_APP_SHUTTER_API environment variable is required but not set");
- }
+ // Prefer VITE_ (Vite). Fallback to REACT_APP_ only if you still ship CRA envs.
+ const SHUTTER_API =
+ import.meta.env.VITE_SHUTTER_API ?? import.meta.env["REACT_APP_SHUTTER_API"];
+ if (!SHUTTER_API) {
+ console.error("Missing Shutter API URL. Set VITE_SHUTTER_API (preferred) or REACT_APP_SHUTTER_API.");
+ // TODO: replace with a user-facing toast/notification if available
+ return;
+ }Notes:
- Use bracket notation for REACT_APP_SHUTTER_API to avoid TS property typing errors.
- Consider hoisting SHUTTER_API to a module-level const to avoid re-evaluating inside the callback (envs are static at build time).
🤖 Prompt for AI Agents
In web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx around lines 88 to
91, the environment variable check uses the incorrect prefix
REACT_APP_SHUTTER_API which is not exposed by Vite at runtime, causing the guard
to always fail. Change the check to use import.meta.env.VITE_SHUTTER_API,
optionally falling back to import.meta.env["REACT_APP_SHUTTER_API"] using
bracket notation to avoid TS errors. Hoist this environment variable to a
module-level constant to prevent repeated evaluation. Instead of throwing an
error inside the UI handler, return early and display a user-facing error
message to avoid unhandled promise rejections.
💡 Verification agent
🧩 Analysis chain
Surface configuration error earlier and disable the action rather than fail on click.
Prefer determining readiness once (e.g., const isShutterReady = Boolean(SHUTTER_API)) and disabling the commit UI or showing an inline warning, instead of throwing during the click handler. This prevents unhandled rejections and provides clearer UX.
You can verify consistency of env usage and whether OptionsContainer supports a disabled or notice prop:
🏁 Script executed:
#!/bin/bash
# Inspect env usage patterns to ensure VITE_ is standard in the repo.
rg -n "import.meta.env\." -S | sed -E 's/.*import\.meta\.env\.([A-Z0-9_]+).*/\1/' | sort -u
# Look for OptionsContainer props to see if we can disable/notify from parent.
fd -t f OptionsContainer | xargs -I{} sh -c 'echo "==> {}"; sed -n "1,200p" "{}"'Length of output: 4625
Compute Shutter readiness up-front & disable commit UI instead of throwing
Rather than throwing in the click handler, derive a single const isShutterReady = Boolean(import.meta.env.REACT_APP_SHUTTER_API) at the top of the component. Then:
- Pass
disabled={!isShutterReady || isLoading}(and optionally an inline tooltip or notice when!isShutterReady) to your commit<Button>. - Optionally render a short warning message above or below the button when
isShutterReadyis false.
This surfaces configuration issues immediately, prevents unhandled rejections on click, and gives the user clearer feedback.
• File: web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
• Replace lines 88–91 with an upfront isShutterReady check and wire it into the button’s disabled/title (or inline notice) props.
🤖 Prompt for AI Agents
In web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx around lines 88 to
91, replace the runtime throw error for missing REACT_APP_SHUTTER_API with a
computed constant isShutterReady at the top of the component. Use this constant
to disable the commit button by passing disabled={!isShutterReady || isLoading}
and optionally add a tooltip or inline warning message near the button when
isShutterReady is false. This change prevents throwing errors on click and
provides immediate user feedback about configuration issues.
|
jaybuidl
approved these changes
Aug 11, 2025
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.
PR-Codex overview
This PR adds a validation check for the
REACT_APP_SHUTTER_APIenvironment variable in thehandleCommitfunction. If the variable is not set or is empty, an error is logged, and an exception is thrown to prevent further execution.Detailed summary
REACT_APP_SHUTTER_APIinhandleCommit.Summary by CodeRabbit