From a0b0288eb46b518a171e46618012da5ec9d1ad64 Mon Sep 17 00:00:00 2001 From: Joaquin Colacci Date: Tue, 28 Nov 2023 16:40:03 +0100 Subject: [PATCH 1/5] fix: remove enters and double or more spaces from the activity log (#161) --- src/providers/activity.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/providers/activity.ts b/src/providers/activity.ts index a8c08e7..b7b843a 100644 --- a/src/providers/activity.ts +++ b/src/providers/activity.ts @@ -47,8 +47,9 @@ class ActivityLogItem extends vscode.TreeItem { constructor(public readonly log: ActivityLog) { // Shorten the displayed query if it's too long const shortSQL = log.sql.length > 50 ? log.sql.substring(0, 50) + "..." : log.sql; - - super(shortSQL, vscode.TreeItemCollapsibleState.None); + // Removes enters and extra spaces. + const cleanSQL = shortSQL.replace(/\n/g, '').replace(/\s{2,}/g, ' '); + super(cleanSQL, vscode.TreeItemCollapsibleState.None); // Set iconPath based on the status const iconName = log.status === "success" ? "success_icon.svg" : "error_icon.svg"; From e1b79d08266caa9e976920e4f9f92ea1f7daeea7 Mon Sep 17 00:00:00 2001 From: Joaquin Colacci Date: Tue, 28 Nov 2023 16:40:19 +0100 Subject: [PATCH 2/5] context: fix error in login after succesful delete (#160) --- src/context/appPassword.ts | 2 +- src/context/asyncContext.ts | 13 ++++++++++--- src/context/config.ts | 19 ++++++++++++++++++- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/context/appPassword.ts b/src/context/appPassword.ts index 1c231ce..0d692d1 100644 --- a/src/context/appPassword.ts +++ b/src/context/appPassword.ts @@ -70,6 +70,6 @@ export default class AppPassword { } } - throw new ExtensionError(Errors.invalidAppPassword, "Invalid length."); + throw new ExtensionError(Errors.invalidAppPassword, "Invalid app-password length."); } } \ No newline at end of file diff --git a/src/context/asyncContext.ts b/src/context/asyncContext.ts index 68ab54c..c3d38e4 100644 --- a/src/context/asyncContext.ts +++ b/src/context/asyncContext.ts @@ -288,9 +288,16 @@ export default class AsyncContext extends Context { */ async removeAndSaveProfile(name: string) { try { - this.config.removeAndSaveProfile(name); - const success = await this.reloadContext(); - return success; + await this.config.removeAndSaveProfile(name); + const leftProfileNames = this.getProfileNames(); + console.log("[AsyncContext]", "Left profile names: ", leftProfileNames); + if (leftProfileNames && leftProfileNames.length > 0) { + const success = await this.reloadContext(); + return success; + } else { + this.isReadyPromise = new Promise((res) => res(true)); + return true; + } } catch (err) { throw this.parseErr(err, "Error reloading context."); } diff --git a/src/context/config.ts b/src/context/config.ts index df74c45..9f8de58 100644 --- a/src/context/config.ts +++ b/src/context/config.ts @@ -207,6 +207,7 @@ export class Config { } this.save(); + await this.removeKeychainPassword(name); if (newProfileName) { this.setProfile(newProfileName); @@ -276,7 +277,6 @@ export class Config { resolve(); } }; - keychain.setPassword({ account, password: appPassword, @@ -286,6 +286,23 @@ export class Config { }); } + /// Removes an app-password from the keychain + async removeKeychainPassword(account: string): Promise { + return new Promise((resolve, reject) => { + const cb = (err: KeychainError): void => { + if (err) { + reject(err); + } else { + resolve(); + } + }; + keychain.deletePassword({ + account, + service: KEYCHAIN_SERVICE, + }, cb); + }); + } + /// Gets a keychain app-password for a profile async getKeychainPassword(account: string): Promise { return new Promise((resolve, reject) => { From 0ebe351e84430683d5f0992c2cbda828930ae92a Mon Sep 17 00:00:00 2001 From: Joaquin Colacci Date: Thu, 7 Dec 2023 10:25:19 -0500 Subject: [PATCH 3/5] fix: reconnect on connection issues (#165) --- src/clients/sql.ts | 98 +++++++++++++++++++++++++++------------------- 1 file changed, 58 insertions(+), 40 deletions(-) diff --git a/src/clients/sql.ts b/src/clients/sql.ts index 0198fbe..a5a18e8 100644 --- a/src/clients/sql.ts +++ b/src/clients/sql.ts @@ -1,9 +1,8 @@ -import { Pool, PoolClient, PoolConfig, QueryArrayResult, QueryResult } from "pg"; +import { Pool, PoolClient, PoolConfig, QueryArrayResult } from "pg"; import AdminClient from "./admin"; import CloudClient from "./cloud"; import { Profile } from "../context/config"; import AsyncContext from "../context/asyncContext"; -import { Errors, ExtensionError } from "../utilities/error"; export default class SqlClient { private pool: Promise; @@ -12,6 +11,7 @@ export default class SqlClient { private cloudClient: CloudClient; private context: AsyncContext; private profile: Profile; + private ended: boolean; constructor( adminClient: AdminClient, @@ -23,45 +23,61 @@ export default class SqlClient { this.cloudClient = cloudClient; this.profile = profile; this.context = context; + this.ended = false; + this.pool = this.buildPool(); + this.privateClient = this.buildPrivateClient(); + this.handleReconnection(); + } - this.pool = new Promise((res, rej) => { - const asyncOp = async () => { - try { - console.log("[SqlClient]", "Building config."); - const config = await this.buildPoolConfig(); - const pool = new Pool(config); - console.log("[SqlClient]", "Connecting pool."); - - pool.connect().then(() => { - console.log("[SqlClient]", "Pool successfully connected."); - res(pool); - }).catch((err) => { - console.error(err); - rej(new ExtensionError(Errors.poolConnectionFailure, err)); - }); - } catch (err) { - console.error("[SqlClient]", "Error creating pool: ", err); - rej(new ExtensionError(Errors.poolCreationFailure, err)); - } - }; - - asyncOp(); - }); + private async handleReconnection() { + let reconnecting = false; + + const reconnect = (err: Error) => { + console.error("[SqlClient]", "Unexpected error: ", err); + console.log("[SqlClient]", "Reconnecting."); + if (reconnecting === false && this.ended === false) { + reconnecting = true; + const interval = setInterval(async () => { + try { + const pool = await this.pool; + pool.end(); + } catch (err) { + console.error("[SqlClient]", "Error awaiting pool to end. It is ok it the pool connection failed."); + } finally { + this.pool = this.buildPool(); + this.privateClient = this.buildPrivateClient(); + this.handleReconnection(); + reconnecting = false; + clearInterval(interval); + } + }, 5000); + } + }; - this.privateClient = new Promise((res, rej) => { - const asyncOp = async () => { - try { - const pool = await this.pool; - const client = await pool.connect(); - res(client); - } catch (err) { - console.error("[SqlClient]", "Error awaiting the pool: ", err); - rej(err); - } - }; - - asyncOp(); - }); + try { + const pool = await this.pool; + pool.on("error", reconnect); + + try { + const client = await this.privateClient; + client.on("error", reconnect); + } catch (err) { + reconnect(err as Error); + console.error("[SqlClient]", "Unexpected error on client: ", err); + } + } catch (err) { + reconnect(err as Error); + console.error("[SqlClient]", "Unexpected error on pool: ", err); + } + } + + private async buildPrivateClient(): Promise { + const pool = await this.pool; + return pool.connect(); + } + + private async buildPool(): Promise { + return new Pool(await this.buildPoolConfig()); } async connectErr() { @@ -124,7 +140,7 @@ export default class SqlClient { * @param values * @returns query results */ - async internalQuery(statement: string, values?: Array): Promise> { + async internalQuery(statement: string, values?: Array): Promise> { const pool = await this.pool; const results = await pool.query(statement, values); @@ -163,6 +179,8 @@ export default class SqlClient { * Shut down cleanly the pool. */ async end() { + this.ended = true; + try { const pool = await this.pool; await pool.end(); From 3765deed07c2fa8b061e15d16e77dec4b8f116ac Mon Sep 17 00:00:00 2001 From: Joaquin Colacci Date: Thu, 7 Dec 2023 12:21:27 -0500 Subject: [PATCH 4/5] refactor: rename materialize-sql to mzsql (#166) --- package.json | 13 ++-- src/clients/lsp.ts | 2 +- ...ialize-sql.tmLanguage => mzsql.tmLanguage} | 68 +++++++++---------- 3 files changed, 42 insertions(+), 41 deletions(-) rename syntaxes/{materialize-sql.tmLanguage => mzsql.tmLanguage} (88%) diff --git a/package.json b/package.json index 8b30368..d2cd3a2 100644 --- a/package.json +++ b/package.json @@ -40,9 +40,10 @@ }, "languages": [ { - "id": "materialize-sql", + "id": "mzsql", "extensions": [ - ".sql" + ".sql", + ".mzsql" ], "aliases": [ "Materialize SQL" @@ -56,9 +57,9 @@ ], "grammars": [ { - "language": "materialize-sql", - "scopeName": "source.materialize-sql", - "path": "./syntaxes/materialize-sql.tmLanguage" + "language": "mzsql", + "scopeName": "source.mzsql", + "path": "./syntaxes/mzsql.tmLanguage" } ], "menus": { @@ -186,7 +187,7 @@ "command": "materialize.run", "key": "ctrl+enter", "mac": "cmd+enter", - "when": "resourceLangId == 'sql' || resourceLangId == 'materialize-sql'" + "when": "resourceLangId == 'sql' || resourceLangId == 'mzsql'" } ] }, diff --git a/src/clients/lsp.ts b/src/clients/lsp.ts index e88ea50..545ffd6 100644 --- a/src/clients/lsp.ts +++ b/src/clients/lsp.ts @@ -261,7 +261,7 @@ export default class LspClient { const formattingWidth = configuration.get('formattingWidth'); console.log("[LSP]", "Formatting width: ", formattingWidth); const clientOptions: LanguageClientOptions = { - documentSelector: [{ scheme: "file", language: "materialize-sql"}], + documentSelector: [{ scheme: "file", language: "mzsql"}], initializationOptions: { formattingWidth, } diff --git a/syntaxes/materialize-sql.tmLanguage b/syntaxes/mzsql.tmLanguage similarity index 88% rename from syntaxes/materialize-sql.tmLanguage rename to syntaxes/mzsql.tmLanguage index 8de966e..3e22ed2 100644 --- a/syntaxes/materialize-sql.tmLanguage +++ b/syntaxes/mzsql.tmLanguage @@ -26,28 +26,28 @@ 1 name - keyword.other.create.materialize-sql + keyword.other.create.mzsql 2 name - keyword.other.materialize-sql + keyword.other.mzsql 3 name - keyword.other.materialize-sql + keyword.other.mzsql 4 name - entity.name.function.materialize-sql + entity.name.function.mzsql end ;\s* name - meta.statement.materialize-sql.create + meta.statement.mzsql.create patterns @@ -80,28 +80,28 @@ 1 name - keyword.other.create.materialize-sql + keyword.other.create.mzsql 2 name - keyword.other.materialize-sql + keyword.other.mzsql 3 name - keyword.other.materialize-sql + keyword.other.mzsql 4 name - entity.name.function.materialize-sql + entity.name.function.mzsql end ;\s* name - meta.statement.materialize-sql.create + meta.statement.mzsql.create patterns @@ -134,7 +134,7 @@ 0 name - keyword.other.materialize-sql + keyword.other.mzsql comment @@ -142,7 +142,7 @@ end ;\s* name - meta.statement.materialize-sql + meta.statement.mzsql patterns @@ -175,7 +175,7 @@ 0 name - meta.preprocessor.materialize-sql + meta.preprocessor.mzsql comment @@ -183,7 +183,7 @@ end \n name - meta.statement.materialize-sql.psql + meta.statement.mzsql.psql repository @@ -198,13 +198,13 @@ 1 name - punctuation.definition.comment.materialize-sql + punctuation.definition.comment.mzsql match (--).*$\n? name - comment.line.double-dash.materialize-sql + comment.line.double-dash.mzsql begin @@ -214,7 +214,7 @@ 0 name - punctuation.definition.comment.materialize-sql + punctuation.definition.comment.mzsql end @@ -237,7 +237,7 @@ end \1 name - meta.dollar-quote.materialize-sql + meta.dollar-quote.mzsql patterns @@ -274,7 +274,7 @@ 1 name - keyword.other.materialize-sql + keyword.other.mzsql match @@ -290,31 +290,31 @@ match \b\d+\b name - constant.numeric.materialize-sql + constant.numeric.mzsql match \* name - keyword.operator.star.materialize-sql + keyword.operator.star.mzsql match [!<>]?=|<>|<|> name - keyword.operator.comparison.materialize-sql + keyword.operator.comparison.mzsql match -|\+|/ name - keyword.operator.math.materialize-sql + keyword.operator.math.mzsql match \|\| name - keyword.operator.concatenator.materialize-sql + keyword.operator.concatenator.mzsql @@ -323,7 +323,7 @@ match \\. name - constant.character.escape.materialize-sql + constant.character.escape.mzsql strings @@ -335,12 +335,12 @@ 1 name - punctuation.definition.string.begin.materialize-sql + punctuation.definition.string.begin.mzsql 3 name - punctuation.definition.string.end.materialize-sql + punctuation.definition.string.end.mzsql comment @@ -348,7 +348,7 @@ match (')[^'\\]*(') name - string.quoted.single.materialize-sql + string.quoted.single.mzsql begin @@ -358,7 +358,7 @@ 0 name - punctuation.definition.string.begin.materialize-sql + punctuation.definition.string.begin.mzsql comment @@ -370,11 +370,11 @@ 0 name - punctuation.definition.string.end.materialize-sql + punctuation.definition.string.end.mzsql name - string.quoted.single.materialize-sql + string.quoted.single.mzsql patterns @@ -389,7 +389,7 @@ match (")[^"#]*(") name - variable.other.materialize-sql + variable.other.mzsql begin @@ -399,13 +399,13 @@ end \1 name - string.unquoted.dollar.materialize-sql + string.unquoted.dollar.mzsql scopeName - source.materialize-sql + source.mzsql uuid 4D6B679D-111C-4529-B558-3F25487D9E27 From e1958c1f45c444972716e9bcc3c53309f52232d4 Mon Sep 17 00:00:00 2001 From: Joaquin Colacci Date: Thu, 7 Dec 2023 13:19:29 -0500 Subject: [PATCH 5/5] test: fetch retry (#162) This PR adds retry to the fetch requests. It also fixes the tests that were having issues when running locally. --- src/clients/admin.ts | 6 ++--- src/clients/cloud.ts | 4 ++-- src/clients/lsp.ts | 8 +++---- src/providers/auth.ts | 3 +-- src/providers/results.ts | 3 +-- src/test/suite/extension.test.ts | 5 ++-- src/test/suite/server.ts | 4 ++-- src/utilities/error.ts | 6 ++++- src/utilities/getNonce.ts | 8 ------- src/utilities/getUri.ts | 5 ---- src/utilities/utils.ts | 41 ++++++++++++++++++++++++++++++++ 11 files changed, 62 insertions(+), 31 deletions(-) delete mode 100644 src/utilities/getNonce.ts delete mode 100644 src/utilities/getUri.ts create mode 100644 src/utilities/utils.ts diff --git a/src/clients/admin.ts b/src/clients/admin.ts index 1e932e0..b457e0b 100644 --- a/src/clients/admin.ts +++ b/src/clients/admin.ts @@ -1,8 +1,8 @@ -import fetch from "node-fetch"; import AppPassword from "../context/appPassword"; import { Errors, ExtensionError } from "../utilities/error"; import jwksClient from "jwks-rsa"; import { verify } from "node-jsonwebtoken"; +import { fetchWithRetry } from "../utilities/utils"; interface AuthenticationResponse { accessToken: string, @@ -44,11 +44,11 @@ export default class AdminClient { secret: this.appPassword.secretKey }; - const response = await fetch(this.adminEndpoint, { + const response = await fetchWithRetry(this.adminEndpoint, { method: 'post', body: JSON.stringify(authRequest), // eslint-disable-next-line @typescript-eslint/naming-convention - headers: {'Content-Type': 'application/json'} + headers: {'Content-Type': 'application/json' } }); if (response.status === 200) { diff --git a/src/clients/cloud.ts b/src/clients/cloud.ts index 253ea0b..4a46822 100644 --- a/src/clients/cloud.ts +++ b/src/clients/cloud.ts @@ -1,6 +1,6 @@ -import fetch from "node-fetch"; import AdminClient from "./admin"; import { Errors, ExtensionError } from "../utilities/error"; +import { fetchWithRetry } from "../utilities/utils"; const DEFAULT_API_CLOUD_ENDPOINT = 'https://api.cloud.materialize.com'; @@ -58,7 +58,7 @@ export default class CloudClient { } async fetch(endpoint: string) { - return fetch(endpoint, { + return fetchWithRetry(endpoint, { method: 'get', headers: { // eslint-disable-next-line @typescript-eslint/naming-convention diff --git a/src/clients/lsp.ts b/src/clients/lsp.ts index 545ffd6..ae1d6e8 100644 --- a/src/clients/lsp.ts +++ b/src/clients/lsp.ts @@ -1,5 +1,4 @@ /* eslint-disable @typescript-eslint/naming-convention */ -import fetch from "node-fetch"; import path from "path"; import * as vscode from "vscode"; import { @@ -16,6 +15,7 @@ import os from "os"; import { SemVer } from "semver"; import { Errors, ExtensionError } from "../utilities/error"; import * as Sentry from "@sentry/node"; +import { fetchWithRetry } from "../utilities/utils"; // This endpoint returns a string with the latest LSP version. const BINARIES_ENDPOINT = "https://binaries.materialize.com"; @@ -144,7 +144,7 @@ export default class LspClient { bufferStream .pipe(gunzip) .pipe(extract) - .on('finish', (d: any) => { + .on('finish', () => { console.log("[LSP]", "Server installed."); res(""); }) @@ -198,7 +198,7 @@ export default class LspClient { */ private async fetchLatestVersionNumber() { console.log("[LSP]", "Fetching latest version number."); - const response = await fetch(LATEST_VERSION_ENDPOINT); + const response = await fetchWithRetry(LATEST_VERSION_ENDPOINT); const latestVersion: string = await response.text(); return new SemVer(latestVersion); @@ -213,7 +213,7 @@ export default class LspClient { const endpoint = this.getEndpointByOs(latestVersion); console.log("[LSP]", `Fetching LSP from: ${endpoint}`); - const binaryResponse = await fetch(endpoint); + const binaryResponse = await fetchWithRetry(endpoint); const buffer = await binaryResponse.arrayBuffer(); return buffer; diff --git a/src/providers/auth.ts b/src/providers/auth.ts index a50ad3b..6110580 100644 --- a/src/providers/auth.ts +++ b/src/providers/auth.ts @@ -1,8 +1,7 @@ import * as vscode from "vscode"; import express, { Request, Response, Application, } from 'express'; -import { getUri } from "../utilities/getUri"; +import { getNonce, getUri } from "../utilities/utils"; import AppPassword from "../context/appPassword"; -import { getNonce } from "../utilities/getNonce"; import AsyncContext from "../context/asyncContext"; import { Errors, ExtensionError } from "../utilities/error"; diff --git a/src/providers/results.ts b/src/providers/results.ts index 924cc4e..88187c7 100644 --- a/src/providers/results.ts +++ b/src/providers/results.ts @@ -1,6 +1,5 @@ import * as vscode from "vscode"; -import { getUri } from "../utilities/getUri"; -import { getNonce } from "../utilities/getNonce"; +import { getNonce, getUri } from "../utilities/utils"; import { QueryResult } from "pg"; interface Results extends QueryResult { diff --git a/src/test/suite/extension.test.ts b/src/test/suite/extension.test.ts index bc4214d..ccb1312 100644 --- a/src/test/suite/extension.test.ts +++ b/src/test/suite/extension.test.ts @@ -255,13 +255,14 @@ suite('Extension Test Suite', () => { let err = false; try { await context.setProfile("invalid_profile"); - } catch (error) { + } catch (error: any) { + assert.ok(error.message === "Invalid authentication"); err = true; } assert.ok(err); - }).timeout(10000); + }).timeout(30000); test('Alternative parser', async () => { const lsp = new LspClient(); diff --git a/src/test/suite/server.ts b/src/test/suite/server.ts index ea6a36f..30d524d 100644 --- a/src/test/suite/server.ts +++ b/src/test/suite/server.ts @@ -80,8 +80,8 @@ export function mockServer(): Promise { }); return new Promise((res) => { - app.listen(3000, 'localhost', () => { - console.log(`Mock server listening at localhost:3000`); + const server = app.listen(3000, 'localhost', () => { + console.log(`Mock server listening at localhost:3000: `, server.listening); res("Loaded."); }); }); diff --git a/src/utilities/error.ts b/src/utilities/error.ts index 89a4851..e403754 100644 --- a/src/utilities/error.ts +++ b/src/utilities/error.ts @@ -149,5 +149,9 @@ export enum Errors { /** * Raises when it is impossible to parse the statements. */ - parsingFailure = " Error parsing the statements.", + parsingFailure = "Error parsing the statements.", + /** + * Raises when a fetch failes after a minute. + */ + fetchTimeoutError = "Failed to fetch after a minute." } diff --git a/src/utilities/getNonce.ts b/src/utilities/getNonce.ts deleted file mode 100644 index 205ef23..0000000 --- a/src/utilities/getNonce.ts +++ /dev/null @@ -1,8 +0,0 @@ -export function getNonce() { - let text = ""; - const possible = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; - for (let i = 0; i < 32; i++) { - text += possible.charAt(Math.floor(Math.random() * possible.length)); - } - return text; - } \ No newline at end of file diff --git a/src/utilities/getUri.ts b/src/utilities/getUri.ts deleted file mode 100644 index f43ed04..0000000 --- a/src/utilities/getUri.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { Uri, Webview } from "vscode"; - -export function getUri(webview: Webview, extensionUri: Uri, pathList: string[]) { - return webview.asWebviewUri(Uri.joinPath(extensionUri, ...pathList)); -} \ No newline at end of file diff --git a/src/utilities/utils.ts b/src/utilities/utils.ts new file mode 100644 index 0000000..a582a0c --- /dev/null +++ b/src/utilities/utils.ts @@ -0,0 +1,41 @@ +import { Uri, Webview } from "vscode"; +import { Errors, ExtensionError } from "./error"; + +export function getNonce() { + let text = ""; + const possible = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; + for (let i = 0; i < 32; i++) { + text += possible.charAt(Math.floor(Math.random() * possible.length)); + } + return text; +} + +export function getUri(webview: Webview, extensionUri: Uri, pathList: string[]) { + return webview.asWebviewUri(Uri.joinPath(extensionUri, ...pathList)); +} + +export async function fetchWithRetry( + url: string, + init?: RequestInit, + timeout = 60000, + interval = 5000 +): Promise { + const startTime = Date.now(); + + async function attemptFetch(): Promise { + try { + const response = await fetch(url, init); + return response; + } catch (error) { + console.error("[Fetch]", "Error fetching: ", error); + if (Date.now() - startTime < timeout) { + await new Promise(resolve => setTimeout(resolve, interval)); + return attemptFetch(); + } else { + throw new ExtensionError(Errors.fetchTimeoutError, `Failed to fetch after ${timeout}ms: ${error}`); + } + } + } + + return attemptFetch(); +} \ No newline at end of file