-
Notifications
You must be signed in to change notification settings - Fork 108
base natural postgres attached to database. #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: starter
Are you sure you want to change the base?
Conversation
|
@awselliottai is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
app/actions.ts
Outdated
| data = await sql.query(query); | ||
| } catch (e: any) { | ||
| if (e.message.includes('relation "unicorns" does not exist')) { | ||
| if (e.message.includes('relation "relation" does not exist')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (e.message.includes('relation "relation" does not exist')) { | |
| if (e.message.includes('relation "unicorns" does not exist')) { |
The table name in the error message check was changed from "unicorns" to "relation", which will prevent the error handler from catching the expected error condition. This is likely a copy-paste error from a template.
View Details
Analysis
Incorrect table name in error handler prevents missing table detection
What fails: The runGeneratedSQLQuery() function fails to catch missing table errors because it checks for 'relation "relation" does not exist' instead of the actual table name 'relation "unicorns" does not exist'
How to reproduce:
- The application only supports queries against the "unicorns" table (see lib/seed.ts)
- Delete or drop the unicorns table from PostgreSQL
- Run any SELECT query through the UI that would query the table
- The error is re-thrown instead of being caught
Result: PostgreSQL returns error message: 'relation "unicorns" does not exist' (includes actual table name per PostgreSQL error format)
The condition e.message.includes('relation "relation" does not exist') never matches, so the error falls through to else { throw e; } and propagates to the user instead of being handled gracefully.
Expected: According to PostgreSQL documentation and error message format specifications, when a table doesn't exist, the error message includes the actual table name: 'relation "unicorns" does not exist'. The check should match this specific error to catch it and handle the missing table condition.
Fix: Changed line 78 from checking for 'relation "relation" does not exist' to the correct 'relation "unicorns" does not exist'
| setQueryExpanded(true); | ||
| setLoadingExplanation(true); | ||
|
|
||
| // TODO: generate explanation and update state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handleExplainQuery function lacks error handling. If explainQuery throws an error, setLoadingExplanation(false) won't be called, leaving the UI in a loading state indefinitely.
View Details
📝 Patch Details
diff --git a/components/query-viewer.tsx b/components/query-viewer.tsx
index 216d2ca..7646f9b 100644
--- a/components/query-viewer.tsx
+++ b/components/query-viewer.tsx
@@ -4,6 +4,7 @@ import { QueryWithTooltips } from "./ui/query-with-tooltips";
import { explainQuery } from '@/app/actions';
import { QueryExplanation } from "@/lib/types";
import { CircleHelp, Loader2 } from "lucide-react";
+import { toast } from "sonner";
export const QueryViewer = ({
activeQuery,
@@ -22,9 +23,15 @@ export const QueryViewer = ({
setQueryExpanded(true);
setLoadingExplanation(true);
-const explanations = await explainQuery(inputValue, activeQuery);
- setQueryExplanations(explanations);
- setLoadingExplanation(false);
+ try {
+ const explanations = await explainQuery(inputValue, activeQuery);
+ setQueryExplanations(explanations);
+ } catch (error) {
+ console.error('Failed to generate explanation:', error);
+ toast.error('Failed to generate explanation. Please try again.');
+ } finally {
+ setLoadingExplanation(false);
+ }
};
if (activeQuery.length === 0) return null;
Analysis
Missing error handling in handleExplainQuery leaves UI in loading state
What fails: QueryViewer.handleExplainQuery() lacks error handling. When explainQuery() throws an error, setLoadingExplanation(false) is never called, leaving the spinner visible indefinitely.
How to reproduce:
- Navigate to the query explanation feature
- Click "Explain query" button when
explainQueryfails (e.g., API error, network failure) - The loading spinner remains visible
- No error message displayed to the user
Result: Unhandled promise rejection; loading state remains true; spinner never stops; user receives no feedback about the failure.
Expected: Errors should be caught, loading state should always be cleared, and user should receive error feedback via toast notification.
Fix implemented: Added try-catch-finally block to handleExplainQuery that:
- Catches errors from
explainQuery() - Displays error toast notification to user
- Always clears loading state in finally block
This follows the same error handling pattern used elsewhere in the codebase (see app/page.tsx for toast error patterns).
| setColumns(columns); | ||
|
|
||
| setLoading(false); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loading state is set to false before the chart config generation completes, causing the UI to show results as fully loaded while still waiting for the chart.
View Details
📝 Patch Details
diff --git a/app/page.tsx b/app/page.tsx
index f5c2dc8..d9cb4ad 100644
--- a/app/page.tsx
+++ b/app/page.tsx
@@ -61,10 +61,10 @@ export default function Page() {
setResults(companies);
setColumns(columns);
- setLoading(false);
+ const { config } = await generateChartConfig(companies, question);
+ setChartConfig(config);
- const { config } = await generateChartConfig(companies, question);
- setChartConfig(config);
+ setLoading(false);
} catch (e) {
toast.error("An error occurred. Please try again.");
setLoading(false);
Analysis
Loading state set before async chart config generation completes
What fails: In app/page.tsx handleSubmit(), setLoading(false) was called before awaiting generateChartConfig(), causing the UI loading spinner to disappear while the chart was still being generated. This creates a confusing UX where the table appears fully loaded but the chart tab shows a skeleton loading state.
How to reproduce:
- Run the application
- Submit a query that generates results
- Observe: Loading spinner disappears, table displays
- Chart tab shows SkeletonCard (loading indicator) while chart config is still being generated
- After ~200-500ms, the chart finally appears
Expected behavior: The loading spinner should remain visible until both the SQL results AND the chart config are ready, then disappear when everything is loaded.
Fix applied: Moved setLoading(false) to occur after await generateChartConfig() completes, ensuring the overall loading state remains true during chart configuration generation.
No description provided.