Make localhost/port values configurable via env vars#84
Open
LuisAPI wants to merge 3 commits into
Open
Conversation
- Use ALLOWED_ORIGINS env var or sensible default list for CORS - Make CLASSIFIER_URL configurable (default http://localhost:5001) - Make frontend REACT_APP_BACKEND_URL fallback dynamic (uses BACKEND_PORT) - Add README docs for new env vars (ALLOWED_ORIGINS, CLASSIFIER_URL, BACKEND_PORT, BACKEND_HOST)
✅ Deploy Preview for imfrisivmail ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Reviewer's GuideThis PR replaces hardcoded localhost and port values across backend and frontend with environment-variable-driven configuration for CORS origins, server host/ports, classifier URLs, and updates README accordingly. Class diagram for updated configuration usage in backend and frontendclassDiagram
class ExpressServer {
+allowedOrigins: string[]
+PORT: number
+HOST: string
+constructor()
+startupLog()
}
class GmailRoute {
+classifierBase: string
+POST /classify
}
class ReactApp {
+backendUrl: string
+frontendUrl: string
+handleGmailLogin()
}
ExpressServer <|-- GmailRoute
ReactApp -- ExpressServer: "Sends login/auth requests"
GmailRoute -- Classifier: "Calls classifier service"
Classifier: "URL configurable via CLASSIFIER_URL"
ExpressServer: "CORS origins configurable via ALLOWED_ORIGINS"
ExpressServer: "Port configurable via BACKEND_PORT"
ExpressServer: "Host configurable via BACKEND_HOST"
ReactApp: "Backend URL configurable via REACT_APP_BACKEND_URL"
GmailRoute: "Classifier URL configurable via CLASSIFIER_URL"
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `backend/server.js:16` </location>
<code_context>
+const allowedOrigins = process.env.ALLOWED_ORIGINS
</code_context>
<issue_to_address>
**suggestion:** Consider validating ALLOWED_ORIGINS input for empty strings.
Empty strings in allowedOrigins may cause unexpected CORS behavior. Filter them out after splitting to improve robustness.
```suggestion
? process.env.ALLOWED_ORIGINS.split(",").map((s) => s.trim()).filter(Boolean)
```
</issue_to_address>
### Comment 2
<location> `backend/server.js:150-152` </location>
<code_context>
};
https.createServer(options, app).listen(PORT, () => {
- console.log(`HTTPS server running at https://localhost:${PORT}`);
+ const host = process.env.BACKEND_HOST || "localhost";
+ console.log(`HTTPS server running at https://${host}:${PORT}`);
});
</code_context>
<issue_to_address>
**suggestion:** BACKEND_HOST may not match actual binding host.
If the server binds to a different host than BACKEND_HOST, the log output may be inaccurate. Consider clarifying the log message or ensuring the binding matches BACKEND_HOST.
```suggestion
https.createServer(options, app).listen(PORT, () => {
// The server binds to 0.0.0.0 or localhost by default unless specified otherwise.
// BACKEND_HOST may be used for external access, but may not match the actual binding host.
const externalHost = process.env.BACKEND_HOST || "localhost";
console.log(`HTTPS server running (bound to port ${PORT}). External access: https://${externalHost}:${PORT}`);
```
</issue_to_address>
### Comment 3
<location> `frontend/src/App.js:41-45` </location>
<code_context>
+ // to localhost on `BACKEND_PORT` (default 5002). In production default to the current origin.
+ const backendUrl =
+ process.env.REACT_APP_BACKEND_URL ||
+ ((window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1')
+ ? `https://${window.location.hostname}:${process.env.BACKEND_PORT || 5002}`
+ : `${window.location.protocol}//${window.location.host}`);
const frontendUrl = window.location.origin;
const handleGmailLogin = () => {
</code_context>
<issue_to_address>
**suggestion:** Mixing protocol and port logic may cause issues in some dev setups.
Defaulting to https for localhost can cause connection errors if the backend uses http. Consider detecting the protocol or making it configurable to support different development setups.
```suggestion
// Allow protocol override for localhost via REACT_APP_BACKEND_PROTOCOL, default to 'http'
const backendProtocol =
process.env.REACT_APP_BACKEND_PROTOCOL ||
((window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1') ? 'http' : window.location.protocol.replace(':', ''));
const backendUrl =
process.env.REACT_APP_BACKEND_URL ||
((window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1')
? `${backendProtocol}://${window.location.hostname}:${process.env.BACKEND_PORT || 5002}`
: `${window.location.protocol}//${window.location.host}`);
```
</issue_to_address>
### Comment 4
<location> `backend/routes/gmail.js:76-79` </location>
<code_context>
- // Call classifier
- const classificationResponse = await axios.post("http://localhost:5001/classify", {
+ // Call classifier (URL configurable via CLASSIFIER_URL env var)
+ const classifierBase = process.env.CLASSIFIER_URL || "http://localhost:5001";
+ const classificationResponse = await axios.post(`${classifierBase.replace(/\/$/,"")}/classify`, {
emails: emailBodies,
</code_context>
<issue_to_address>
**suggestion:** Consider validating CLASSIFIER_URL for trailing slashes.
Also, trim whitespace from CLASSIFIER_URL to prevent issues from accidental spaces or formatting.
```suggestion
let classifierBase = process.env.CLASSIFIER_URL || "http://localhost:5001";
classifierBase = classifierBase.trim().replace(/\/+$/, "");
const classificationResponse = await axios.post(`${classifierBase}/classify`, {
emails: emailBodies,
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // Allow configuring allowed CORS origins via environment variable `ALLOWED_ORIGINS` as a comma-separated list. | ||
| // If not provided, fall back to a safe default set used for local development. | ||
| const allowedOrigins = process.env.ALLOWED_ORIGINS | ||
| ? process.env.ALLOWED_ORIGINS.split(",").map((s) => s.trim()) |
There was a problem hiding this comment.
suggestion: Consider validating ALLOWED_ORIGINS input for empty strings.
Empty strings in allowedOrigins may cause unexpected CORS behavior. Filter them out after splitting to improve robustness.
Suggested change
| ? process.env.ALLOWED_ORIGINS.split(",").map((s) => s.trim()) | |
| ? process.env.ALLOWED_ORIGINS.split(",").map((s) => s.trim()).filter(Boolean) |
Comment on lines
146
to
+152
| https.createServer(options, app).listen(PORT, () => { | ||
| console.log(`HTTPS server running at https://localhost:${PORT}`); | ||
| const host = process.env.BACKEND_HOST || "localhost"; | ||
| console.log(`HTTPS server running at https://${host}:${PORT}`); |
There was a problem hiding this comment.
suggestion: BACKEND_HOST may not match actual binding host.
If the server binds to a different host than BACKEND_HOST, the log output may be inaccurate. Consider clarifying the log message or ensuring the binding matches BACKEND_HOST.
Suggested change
| https.createServer(options, app).listen(PORT, () => { | |
| console.log(`HTTPS server running at https://localhost:${PORT}`); | |
| const host = process.env.BACKEND_HOST || "localhost"; | |
| console.log(`HTTPS server running at https://${host}:${PORT}`); | |
| https.createServer(options, app).listen(PORT, () => { | |
| // The server binds to 0.0.0.0 or localhost by default unless specified otherwise. | |
| // BACKEND_HOST may be used for external access, but may not match the actual binding host. | |
| const externalHost = process.env.BACKEND_HOST || "localhost"; | |
| console.log(`HTTPS server running (bound to port ${PORT}). External access: https://${externalHost}:${PORT}`); |
Comment on lines
+41
to
+45
| const backendUrl = | ||
| process.env.REACT_APP_BACKEND_URL || | ||
| ((window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1') | ||
| ? `https://${window.location.hostname}:${process.env.BACKEND_PORT || 5002}` | ||
| : `${window.location.protocol}//${window.location.host}`); |
There was a problem hiding this comment.
suggestion: Mixing protocol and port logic may cause issues in some dev setups.
Defaulting to https for localhost can cause connection errors if the backend uses http. Consider detecting the protocol or making it configurable to support different development setups.
Suggested change
| const backendUrl = | |
| process.env.REACT_APP_BACKEND_URL || | |
| ((window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1') | |
| ? `https://${window.location.hostname}:${process.env.BACKEND_PORT || 5002}` | |
| : `${window.location.protocol}//${window.location.host}`); | |
| // Allow protocol override for localhost via REACT_APP_BACKEND_PROTOCOL, default to 'http' | |
| const backendProtocol = | |
| process.env.REACT_APP_BACKEND_PROTOCOL || | |
| ((window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1') ? 'http' : window.location.protocol.replace(':', '')); | |
| const backendUrl = | |
| process.env.REACT_APP_BACKEND_URL || | |
| ((window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1') | |
| ? `${backendProtocol}://${window.location.hostname}:${process.env.BACKEND_PORT || 5002}` | |
| : `${window.location.protocol}//${window.location.host}`); |
Comment on lines
+76
to
79
| const classifierBase = process.env.CLASSIFIER_URL || "http://localhost:5001"; | ||
| const classificationResponse = await axios.post(`${classifierBase.replace(/\/$/,"")}/classify`, { | ||
| emails: emailBodies, | ||
| }); |
There was a problem hiding this comment.
suggestion: Consider validating CLASSIFIER_URL for trailing slashes.
Also, trim whitespace from CLASSIFIER_URL to prevent issues from accidental spaces or formatting.
Suggested change
| const classifierBase = process.env.CLASSIFIER_URL || "http://localhost:5001"; | |
| const classificationResponse = await axios.post(`${classifierBase.replace(/\/$/,"")}/classify`, { | |
| emails: emailBodies, | |
| }); | |
| let classifierBase = process.env.CLASSIFIER_URL || "http://localhost:5001"; | |
| classifierBase = classifierBase.trim().replace(/\/+$/, ""); | |
| const classificationResponse = await axios.post(`${classifierBase}/classify`, { | |
| emails: emailBodies, | |
| }); |
This was
linked to
issues
Dec 4, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Replaces hardcoded localhost and port values with environment-configurable options and documents them in README.
Changes:
This makes local host/port values configurable for different dev setups and avoids hardcoded redirect/URL values that caused problems when running on different ports or environments.
Summary by Sourcery
Enable configuring backend host, port, CORS origins, and classifier service URL via environment variables, and update frontend to respect REACT_APP_BACKEND_URL with dynamic fallback.
Enhancements:
Documentation: