Skip to content

Commit 368df3d

Browse files
Ajit Pratap Singhclaude
authored andcommitted
fix: address all code review feedback for VSCode extension
Security & Reliability fixes: - Use stdin for SQL content in analyzeCommand (prevents cmd line injection/length issues) - Add executable validation before LSP server start - Add LSP server retry mechanism with exponential backoff (3 retries) - Add 30-second timeout for analyze command to prevent hanging - Increase output buffer to 5MB for large analysis results UX improvements: - Improve validateCommand with proper diagnostic counting - Show progress indicator during analysis - Better status bar feedback (error/retry/running states) - Open Problems panel when validation finds issues - Remove broken image reference from README - Update license reference to AGPL-3.0 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d69db61 commit 368df3d

2 files changed

Lines changed: 154 additions & 63 deletions

File tree

vscode-extension/README.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ High-performance SQL parsing, validation, formatting, and analysis powered by [G
55
## Features
66

77
### Real-time SQL Validation
8-
Get instant feedback on SQL syntax errors as you type. Errors are highlighted directly in the editor with detailed messages.
9-
10-
![Validation](images/validation.gif)
8+
Get instant feedback on SQL syntax errors as you type. Errors are highlighted directly in the editor with detailed messages in the Problems panel.
119

1210
### SQL Formatting
1311
Format your SQL code with customizable indentation and keyword casing.
@@ -118,7 +116,7 @@ Contributions are welcome! Please see our [Contributing Guide](https://github.co
118116

119117
## License
120118

121-
MIT License - see [LICENSE](https://github.com/ajitpratap0/GoSQLX/blob/main/LICENSE)
119+
GNU Affero General Public License v3.0 (AGPL-3.0) - see [LICENSE](https://github.com/ajitpratap0/GoSQLX/blob/main/LICENSE)
122120

123121
## Release Notes
124122

vscode-extension/src/extension.ts

Lines changed: 152 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,45 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
6262
outputChannel.appendLine('GoSQLX extension activated');
6363
}
6464

65-
async function startLanguageServer(context: vscode.ExtensionContext): Promise<void> {
65+
// Validate that the gosqlx executable exists and is runnable
66+
async function validateExecutable(executablePath: string): Promise<boolean> {
67+
return new Promise<boolean>((resolve) => {
68+
const child = spawn(executablePath, ['--version'], { stdio: 'pipe' });
69+
const timeout = setTimeout(() => {
70+
child.kill();
71+
resolve(false);
72+
}, 5000); // 5 second timeout
73+
74+
child.on('close', (code) => {
75+
clearTimeout(timeout);
76+
resolve(code === 0);
77+
});
78+
79+
child.on('error', () => {
80+
clearTimeout(timeout);
81+
resolve(false);
82+
});
83+
});
84+
}
85+
86+
async function startLanguageServer(context: vscode.ExtensionContext, retryCount: number = 0): Promise<void> {
6687
const config = vscode.workspace.getConfiguration('gosqlx');
6788
const executablePath = config.get<string>('executablePath', 'gosqlx');
89+
const maxRetries = 3;
90+
91+
// Validate executable before starting
92+
const isValid = await validateExecutable(executablePath);
93+
if (!isValid) {
94+
const message = `GoSQLX executable not found or not working: ${executablePath}`;
95+
outputChannel.appendLine(message);
96+
vscode.window.showErrorMessage(
97+
`${message}. Please install gosqlx: go install github.com/ajitpratap0/GoSQLX/cmd/gosqlx@latest`
98+
);
99+
statusBarItem.text = '$(error) GoSQLX';
100+
statusBarItem.tooltip = 'GoSQLX: Executable not found';
101+
statusBarItem.show();
102+
return;
103+
}
68104

69105
// Server options - spawn the gosqlx lsp command
70106
// Use cross-platform temp directory for debug logs
@@ -113,17 +149,34 @@ async function startLanguageServer(context: vscode.ExtensionContext): Promise<vo
113149

114150
try {
115151
await client.start();
152+
statusBarItem.text = '$(database) GoSQLX';
153+
statusBarItem.tooltip = 'GoSQLX Language Server - Running';
116154
statusBarItem.show();
117155
outputChannel.appendLine('GoSQLX Language Server started successfully');
118156
vscode.window.showInformationMessage('GoSQLX Language Server started');
119157
} catch (error) {
120158
const message = error instanceof Error ? error.message : String(error);
121159
outputChannel.appendLine(`Failed to start GoSQLX Language Server: ${message}`);
160+
161+
// Retry logic
162+
if (retryCount < maxRetries) {
163+
const retryDelay = Math.pow(2, retryCount) * 1000; // Exponential backoff
164+
outputChannel.appendLine(`Retrying in ${retryDelay / 1000} seconds... (attempt ${retryCount + 1}/${maxRetries})`);
165+
statusBarItem.text = '$(sync~spin) GoSQLX';
166+
statusBarItem.tooltip = `GoSQLX: Retrying... (${retryCount + 1}/${maxRetries})`;
167+
statusBarItem.show();
168+
169+
await new Promise(resolve => setTimeout(resolve, retryDelay));
170+
return startLanguageServer(context, retryCount + 1);
171+
}
172+
122173
vscode.window.showErrorMessage(
123-
`Failed to start GoSQLX Language Server: ${message}. ` +
174+
`Failed to start GoSQLX Language Server after ${maxRetries} attempts: ${message}. ` +
124175
'Make sure gosqlx is installed and in your PATH.'
125176
);
126-
statusBarItem.hide();
177+
statusBarItem.text = '$(error) GoSQLX';
178+
statusBarItem.tooltip = 'GoSQLX: Failed to start';
179+
statusBarItem.show();
127180
}
128181
}
129182

@@ -166,27 +219,44 @@ async function validateCommand(): Promise<void> {
166219
outputChannel.appendLine(`Validating: ${uri.fsPath}`);
167220

168221
try {
169-
// Force re-validation by making a small edit and undoing it
170-
// This triggers the LSP to re-analyze the document
171-
const document = editor.document;
172-
173-
// Alternative: Request diagnostics refresh if the LSP supports it
174-
// Send a didChange notification to trigger revalidation
175-
await vscode.commands.executeCommand('editor.action.triggerSuggest');
176-
await new Promise(resolve => setTimeout(resolve, 100));
222+
// Force document sync by saving if dirty, or touch the document
223+
if (editor.document.isDirty) {
224+
// If document has unsaved changes, the LSP should already have diagnostics
225+
// Just wait a moment for any pending diagnostics
226+
await new Promise(resolve => setTimeout(resolve, 200));
227+
}
177228

178229
// Get current diagnostics for the document
179230
const diagnostics = vscode.languages.getDiagnostics(uri);
180-
const sqlDiagnostics = diagnostics.filter(d => d.source === 'gosqlx' || d.source === 'GoSQLX');
231+
const sqlDiagnostics = diagnostics.filter(d =>
232+
d.source === 'gosqlx' || d.source === 'GoSQLX' || d.source === undefined
233+
);
181234

182235
if (sqlDiagnostics.length === 0) {
183236
vscode.window.showInformationMessage('SQL validation complete. No issues found.');
184237
} else {
185238
const errorCount = sqlDiagnostics.filter(d => d.severity === vscode.DiagnosticSeverity.Error).length;
186239
const warningCount = sqlDiagnostics.filter(d => d.severity === vscode.DiagnosticSeverity.Warning).length;
187-
vscode.window.showWarningMessage(
188-
`SQL validation found ${errorCount} error(s) and ${warningCount} warning(s). Check the Problems panel.`
189-
);
240+
const infoCount = sqlDiagnostics.length - errorCount - warningCount;
241+
242+
let message = 'SQL validation complete: ';
243+
const parts: string[] = [];
244+
if (errorCount > 0) { parts.push(`${errorCount} error(s)`); }
245+
if (warningCount > 0) { parts.push(`${warningCount} warning(s)`); }
246+
if (infoCount > 0) { parts.push(`${infoCount} info`); }
247+
248+
if (parts.length === 0) {
249+
vscode.window.showInformationMessage('SQL validation complete. No issues found.');
250+
} else if (errorCount > 0) {
251+
vscode.window.showErrorMessage(`${message}${parts.join(', ')}. Check the Problems panel.`);
252+
} else {
253+
vscode.window.showWarningMessage(`${message}${parts.join(', ')}. Check the Problems panel.`);
254+
}
255+
}
256+
257+
// Focus the Problems panel if there are issues
258+
if (sqlDiagnostics.length > 0) {
259+
await vscode.commands.executeCommand('workbench.actions.view.problems');
190260
}
191261
} catch (error) {
192262
const message = error instanceof Error ? error.message : String(error);
@@ -228,61 +298,84 @@ async function analyzeCommand(): Promise<void> {
228298
const config = vscode.workspace.getConfiguration('gosqlx');
229299
const executablePath = config.get<string>('executablePath', 'gosqlx');
230300

231-
try {
232-
// Use spawn with argument array to prevent command injection
233-
const result = await new Promise<{ stdout: string; stderr: string }>((resolve, reject) => {
234-
const child = spawn(executablePath, ['analyze', text]);
235-
236-
let stdout = '';
237-
let stderr = '';
238-
let outputSize = 0;
239-
const maxSize = 1024 * 1024; // 1MB limit
240-
241-
if (child.stdout) {
242-
child.stdout.on('data', (data: Buffer) => {
243-
outputSize += data.length;
244-
if (outputSize < maxSize) {
245-
stdout += data.toString();
301+
// Show progress indicator
302+
await vscode.window.withProgress({
303+
location: vscode.ProgressLocation.Notification,
304+
title: 'Analyzing SQL...',
305+
cancellable: false
306+
}, async () => {
307+
try {
308+
// Use stdin to send SQL content - safer than command line arguments
309+
const result = await new Promise<{ stdout: string; stderr: string }>((resolve, reject) => {
310+
const child = spawn(executablePath, ['analyze'], {
311+
stdio: ['pipe', 'pipe', 'pipe']
312+
});
313+
314+
let stdout = '';
315+
let stderr = '';
316+
let outputSize = 0;
317+
const maxSize = 5 * 1024 * 1024; // 5MB limit for large analysis results
318+
319+
// Set a timeout to prevent hanging
320+
const timeout = setTimeout(() => {
321+
child.kill();
322+
reject(new Error('Analysis timed out after 30 seconds'));
323+
}, 30000);
324+
325+
if (child.stdout) {
326+
child.stdout.on('data', (data: Buffer) => {
327+
outputSize += data.length;
328+
if (outputSize < maxSize) {
329+
stdout += data.toString();
330+
}
331+
});
332+
}
333+
334+
if (child.stderr) {
335+
child.stderr.on('data', (data: Buffer) => {
336+
stderr += data.toString();
337+
});
338+
}
339+
340+
child.on('close', (code: number | null) => {
341+
clearTimeout(timeout);
342+
if (code === 0 || code === null) {
343+
resolve({ stdout, stderr });
344+
} else {
345+
reject(new Error(`Process exited with code ${code}: ${stderr || 'Unknown error'}`));
246346
}
247347
});
248-
}
249348

250-
if (child.stderr) {
251-
child.stderr.on('data', (data: Buffer) => {
252-
stderr += data.toString();
349+
child.on('error', (err: Error) => {
350+
clearTimeout(timeout);
351+
reject(err);
253352
});
254-
}
255353

256-
child.on('close', (code: number | null) => {
257-
if (code === 0 || code === null) {
258-
resolve({ stdout, stderr });
259-
} else {
260-
reject(new Error(`Process exited with code ${code}: ${stderr}`));
354+
// Send SQL content via stdin - avoids command line length limits and injection risks
355+
if (child.stdin) {
356+
child.stdin.write(text);
357+
child.stdin.end();
261358
}
262359
});
263360

264-
child.on('error', (err: Error) => {
265-
reject(err);
361+
if (result.stderr) {
362+
outputChannel.appendLine(`Analysis stderr: ${result.stderr}`);
363+
}
364+
365+
// Show analysis results in a new document
366+
const doc = await vscode.workspace.openTextDocument({
367+
content: result.stdout || 'No analysis output',
368+
language: 'markdown'
266369
});
267-
});
370+
await vscode.window.showTextDocument(doc, { preview: true });
268371

269-
if (result.stderr) {
270-
outputChannel.appendLine(`Analysis stderr: ${result.stderr}`);
372+
outputChannel.appendLine(`Analyzed: ${editor.document.uri.fsPath}`);
373+
} catch (error) {
374+
const message = error instanceof Error ? error.message : String(error);
375+
vscode.window.showErrorMessage(`Analysis failed: ${message}`);
376+
outputChannel.appendLine(`Analysis error: ${message}`);
271377
}
272-
273-
// Show analysis results in a new document
274-
const doc = await vscode.workspace.openTextDocument({
275-
content: result.stdout || 'No analysis output',
276-
language: 'markdown'
277-
});
278-
await vscode.window.showTextDocument(doc, { preview: true });
279-
280-
outputChannel.appendLine(`Analyzed: ${editor.document.uri.fsPath}`);
281-
} catch (error) {
282-
const message = error instanceof Error ? error.message : String(error);
283-
vscode.window.showErrorMessage(`Analysis failed: ${message}`);
284-
outputChannel.appendLine(`Analysis error: ${message}`);
285-
}
378+
});
286379
}
287380

288381
export async function deactivate(): Promise<void> {

0 commit comments

Comments
 (0)