diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3eba3ff1..77c60833 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -8,6 +8,41 @@ on: workflow_dispatch: jobs: + # Lint runs as its own job, first and cheapest. It needs no SQL Server, no ODBC + # driver and no native build, so `npm ci --ignore-scripts` is enough and it + # finishes in well under a minute. + # + # It exists because `npm run lint` was broken for weeks without anyone + # noticing: the script was never wired into CI, so a TypeScript major bump that + # made typescript-eslint refuse to load cost nothing and went unseen. Lint that + # nothing runs is lint that rots. + lint: + name: Lint + runs-on: ubuntu-latest + + steps: + - name: Create directory structure + run: mkdir -p node_modules + + - uses: actions/checkout@v7 + with: + path: node_modules/msnodesqlv8 + + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: 22 + + # --ignore-scripts skips the native build: eslint never loads the addon, and + # building it here would add minutes for no benefit. + - name: Install dependencies + working-directory: node_modules/msnodesqlv8 + run: npm ci --ignore-scripts + + - name: Lint + working-directory: node_modules/msnodesqlv8 + run: npm run lint + test-linux: name: Test Linux - Node ${{ matrix.node }} strategy: diff --git a/eslint.config.mjs b/eslint.config.mjs index 03c0c4d0..e0ca6424 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -85,12 +85,31 @@ export default [ '@typescript-eslint/no-empty-object-type': 'off', '@typescript-eslint/no-empty-function': 'off', '@typescript-eslint/no-deprecated': 'off', + // Harness and sample code rethrows errors straight from driver + // callbacks. QueryCb types err as `Error | undefined`, but the + // overloaded query(sqlOrQuery, paramsOrCb?, cb?) signature leaves it + // unresolvable at some call sites, and test/env/cmd-test.ts declares + // several of its own handlers as `err: any`. The rule then cannot + // prove the thrown value is an Error even where a `!= null` guard + // precedes it. Off for the same reason the no-unsafe-* rules above + // are off - the driver's own surface is loosely typed. + '@typescript-eslint/only-throw-error': 'off', + // `res![0]` in a test that has just inserted the row it selects. + // Consistent with consistent-type-assertions being off above. + '@typescript-eslint/no-non-null-assertion': 'off', '@typescript-eslint/no-unnecessary-type-arguments': 'off', '@typescript-eslint/no-unnecessary-type-parameters': 'off', '@typescript-eslint/no-unnecessary-type-conversion': 'off', '@typescript-eslint/no-base-to-string': 'off', '@typescript-eslint/prefer-optional-chain': 'off', '@typescript-eslint/prefer-for-of': 'off', + // Its autofix rewrites Object.prototype.hasOwnProperty.call(o, k) to + // Object.hasOwn(o, k), which does not compile here: tsconfig sets + // target es6 with no explicit lib, and Object.hasOwn is es2022, so tsc + // fails with TS2550. Raising the target is the real fix but it changes + // emitted dist/ output and the generated declarations, so it wants its + // own change rather than riding along with lint wiring. + 'prefer-object-has-own': 'off', 'no-plusplus': 'off', 'no-console': 'off', 'no-param-reassign': 'off', @@ -119,6 +138,39 @@ export default [ '@eslint-community/eslint-comments/no-unlimited-disable': 'off' } }, + { + // lib/ is the shipped JavaScript and had NO lint coverage at all: the block + // above targets lib/**/*.ts, and the only .ts in lib is index.d.ts, which + // the same block then ignores. So 26 product files were never linted. + // + // Stylistic rules only. eslint-config-love is type-aware and expects a + // tsconfig project; pointing it at plain .js files would need type info + // these files do not have. This at least holds the shipped code to the same + // formatting as everything else. + files: ['lib/**/*.js'], + plugins: { '@stylistic': stylistic }, + rules: { + '@stylistic/semi': ['error', 'never'], + '@stylistic/no-trailing-spaces': 'error', + '@stylistic/eol-last': ['error', 'always'], + '@stylistic/no-multiple-empty-lines': ['error', { max: 1, maxEOF: 0 }], + '@stylistic/quotes': ['error', 'single', { avoidEscape: true }], + '@stylistic/comma-dangle': ['error', 'never'], + '@stylistic/space-before-function-paren': ['error', 'always'], + '@stylistic/keyword-spacing': 'error', + '@stylistic/space-infix-ops': 'error', + '@stylistic/comma-spacing': 'error', + '@stylistic/brace-style': ['error', '1tbs', { allowSingleLine: true }], + '@stylistic/block-spacing': 'error', + '@stylistic/key-spacing': 'error', + '@stylistic/space-before-blocks': 'error', + '@stylistic/no-multi-spaces': 'error', + '@stylistic/object-curly-spacing': ['error', 'always'], + '@stylistic/array-bracket-spacing': ['error', 'never'], + '@stylistic/spaced-comment': ['error', 'always'], + '@stylistic/indent': ['error', 2] + } + }, { ignores: ['dist/**', 'node_modules/**'] } diff --git a/lib/notifier.js b/lib/notifier.js index b14503e7..f7a7cd9a 100755 --- a/lib/notifier.js +++ b/lib/notifier.js @@ -222,7 +222,7 @@ const notifyModule = ((() => { if (this.timeoutHandle) { clearTimeout(this.timeoutHandle) } - + if (timeoutMs && timeoutMs > 0) { this.timeoutHandle = setTimeout(() => { logger.debugLazy(() => `Timeout ${timeoutMs}ms reached for query ${this.queryId} - triggering cancel`) diff --git a/lib/sql-client.js b/lib/sql-client.js index 424912fe..4d3da08b 100755 --- a/lib/sql-client.js +++ b/lib/sql-client.js @@ -114,9 +114,9 @@ class SqlClient { getConnectObject (p) { return typeof p === 'string' ? { - conn_str: p, - connect_timeout: 0 - } + conn_str: p, + connect_timeout: 0 + } : p } diff --git a/package.json b/package.json index 968cdac7..db630413 100644 --- a/package.json +++ b/package.json @@ -96,8 +96,8 @@ "test:cpp": "npm run configure:cpp-tests && npm run build:cpp-tests && cd test/cpp/build && ctest -C Release -V", "test:cpp:debug": "npm run configure:cpp-tests && cmake --build test/cpp/build --config Debug -j 8 && cd test/cpp/build && ctest -C Debug -V", "test:cpp:debug:exe": "test/cpp/build/cpp_tests --gtest_filter=\"*\"", - "lint": "eslint \"test/**/*.ts\" \"samples/**/*.ts\"", - "lint:fix": "eslint \"test/**/*.ts\" \"samples/**/*.ts\" --fix", + "lint": "eslint \"lib/**/*.js\" \"test/**/*.ts\" \"samples/**/*.ts\"", + "lint:fix": "eslint \"lib/**/*.js\" \"test/**/*.ts\" \"samples/**/*.ts\" --fix", "test:cpp:clean": "rimraf test/cpp/build && npm run configure:cpp-tests && cmake --build test/cpp/build --config Debug -j 8", "test:cpp:clean:vs": "rimraf test/cpp/build && npm run configure:cpp-tests:vs && cmake --build test/cpp/build --config Debug -j 8", "build:debug": "node-gyp rebuild --debug --verbose -j 4", diff --git a/samples/typescript/logger-example.ts b/samples/typescript/logger-example.ts index e0a23faa..7d9a36f0 100644 --- a/samples/typescript/logger-example.ts +++ b/samples/typescript/logger-example.ts @@ -17,26 +17,26 @@ sql.logger.configureForDevelopment() // Replace with your connection string const connectionString = 'Driver={ODBC Driver 18 for SQL Server};Server=127.0.0.1,1433;Database=node;UID=node_user;PWD=StrongPassword123!;TrustServerCertificate=yes;;Connect Timeout=10' -async function main(): Promise { +async function main (): Promise { let connection try { // The logger will automatically log connection attempts connection = await sql.promises.open(connectionString) - + // Simple query const result: QueryAggregatorResults = await connection.promises.query( 'SELECT name, create_date FROM sys.tables WHERE type = ?', - ['U'] // 'U' for user tables + ['U'] // 'U' for user tables ) - + console.log(`Found ${result.rows} tables`) console.log('First few tables:', result.first?.slice(0, 3)) - + // Stored procedure call (if you have one) // const procResult = await connection.promises.callProc('sp_who') // console.log('Active connections:', procResult.first?.length) - + } catch (error) { // Errors are automatically logged at ERROR level console.error('Database operation failed:', error) @@ -48,7 +48,7 @@ async function main(): Promise { } // Using connection pool with logging -async function poolExample(): Promise { +async function poolExample (): Promise { const pool = new sql.Pool({ connectionString, floor: 2, @@ -57,16 +57,16 @@ async function poolExample(): Promise { try { await pool.promises.open() - + // Execute multiple queries - the pool and logger will show which connection handles each const results = await Promise.all([ pool.promises.query('SELECT 1 as num'), pool.promises.query('SELECT 2 as num'), pool.promises.query('SELECT 3 as num') ]) - + console.log('Pool query results:', results.map(r => r.first?.[0])) - + } finally { await pool.promises.close() } @@ -76,11 +76,16 @@ async function poolExample(): Promise { (async () => { console.log('=== Single Connection Example ===') await main() - + console.log('\n=== Connection Pool Example ===') await poolExample() - + // Get final logger stats const config = sql.logger.getConfiguration() console.log('\nLogger configuration:', config) -})() \ No newline at end of file +})().catch(e => { + // Without this the IIFE is a floating promise: any failure above becomes + // an unhandled rejection, exiting with a bare stack trace and no context. + console.error(e) + process.exitCode = 1 +}) diff --git a/samples/typescript/pooling.ts b/samples/typescript/pooling.ts index 78f3dd45..38b99773 100755 --- a/samples/typescript/pooling.ts +++ b/samples/typescript/pooling.ts @@ -34,7 +34,7 @@ function submit (sql: string): Query { console.log(`send ${timeStr}, sql = ${sql}`) q.on('submitted', (d: QueryDescription) => { console.log(`query submitted ${timeStr}, sql = ${d.query_str}`) - q.on('done', () => console.log(`query done ${timeStr}`)) + q.on('done', () => { console.log(`query done ${timeStr}`) }) }) return q } diff --git a/samples/typescript/simple-logging-promises.ts b/samples/typescript/simple-logging-promises.ts index 1b31fbee..51b729b3 100644 --- a/samples/typescript/simple-logging-promises.ts +++ b/samples/typescript/simple-logging-promises.ts @@ -27,30 +27,30 @@ console.log('Current logger configuration:', config) // Example connection string - replace with your own const connectionString = 'Driver={ODBC Driver 18 for SQL Server};Server=127.0.0.1,1433;Database=node;UID=node_user;PWD=StrongPassword123!;TrustServerCertificate=yes;;Connect Timeout=10' -async function demonstrateLogging(): Promise { +async function demonstrateLogging (): Promise { try { // Log custom messages at different levels sql.logger.trace('Starting promise-based SQL operations', 'demonstrateLogging') sql.logger.debug('Connection string configured', 'demonstrateLogging') - + // Open connection using promises sql.logger.info('Opening database connection...', 'demonstrateLogging') const connection = await sql.promises.open(connectionString) sql.logger.info('Connection opened successfully', 'demonstrateLogging') - + // Execute a simple query sql.logger.debug('Executing test query', 'demonstrateLogging') const results: QueryAggregatorResults = await connection.promises.query('SELECT 1 as test, GETDATE() as currentTime') - + sql.logger.info(`Query completed in ${results.elapsed}ms`, 'demonstrateLogging') sql.logger.debug(`Results: ${JSON.stringify(results.first)}`, 'demonstrateLogging') - + // Demonstrate lazy logging for expensive operations sql.logger.traceLazy(() => { // This function is only called if TRACE level is enabled return `Detailed results: ${JSON.stringify(results, null, 2)}` }, 'demonstrateLogging') - + // Execute multiple queries using pool sql.logger.info('Creating connection pool...', 'demonstrateLogging') const pool = new sql.Pool({ @@ -61,10 +61,10 @@ async function demonstrateLogging(): Promise { heartbeatSql: 'SELECT @@SPID as spid', inactivityTimeoutSecs: 60 }) - + await pool.promises.open() sql.logger.info('Pool opened successfully', 'demonstrateLogging') - + // Execute queries in parallel sql.logger.debug('Executing parallel queries', 'demonstrateLogging') const parallelQueries = Promise.all([ @@ -72,19 +72,19 @@ async function demonstrateLogging(): Promise { pool.promises.query('SELECT 2 as query2'), pool.promises.query('SELECT 3 as query3') ]) - + const parallelResults = await parallelQueries sql.logger.info(`Completed ${parallelResults.length} parallel queries`, 'demonstrateLogging') - + // Clean up sql.logger.debug('Closing pool', 'demonstrateLogging') await pool.promises.close() - + sql.logger.debug('Closing connection', 'demonstrateLogging') await connection.promises.close() - + sql.logger.info('All operations completed successfully', 'demonstrateLogging') - + } catch (error) { sql.logger.error(`Operation failed: ${error}`, 'demonstrateLogging') throw error @@ -103,4 +103,4 @@ demonstrateLogging() .catch((error) => { console.error('Logging demonstration failed:', error) process.exit(1) - }) \ No newline at end of file + }) diff --git a/test/stress/stress-test-runner.ts b/test/stress/stress-test-runner.ts index be1c3f7d..c7b50a59 100644 --- a/test/stress/stress-test-runner.ts +++ b/test/stress/stress-test-runner.ts @@ -27,14 +27,14 @@ interface TestScenario { } class StressTestRunner { - private config: Required + private readonly config: Required private startTime: number | null = null private memoryBaseline: MemoryUsage | null = null private iterationCount = 0 private errors: TestError[] = [] private connection: any = null - constructor(config: StressTestConfig = {}) { + constructor (config: StressTestConfig = {}) { this.config = { connectionString: config.connectionString || process.env.CONNECTION_STRING || '', iterations: config.iterations || 10000, @@ -43,7 +43,7 @@ class StressTestRunner { } } - private getMemoryUsage(): MemoryUsage { + private getMemoryUsage (): MemoryUsage { const usage = process.memoryUsage() return { rss: (usage.rss / 1024 / 1024).toFixed(2), @@ -54,7 +54,7 @@ class StressTestRunner { } } - private logMemory(message = ''): void { + private logMemory (message = ''): void { const memory = this.getMemoryUsage() console.log(`[Memory] ${message}`) console.log(` RSS: ${memory.rss} MB`) @@ -62,7 +62,7 @@ class StressTestRunner { console.log(` Heap Used: ${memory.heapUsed} MB`) console.log(` External: ${memory.external} MB`) console.log(` Array Buffers: ${memory.arrayBuffers} MB`) - + if (this.memoryBaseline) { const heapDelta = (parseFloat(memory.heapUsed) - parseFloat(this.memoryBaseline.heapUsed)).toFixed(2) const rssDelta = (parseFloat(memory.rss) - parseFloat(this.memoryBaseline.rss)).toFixed(2) @@ -72,7 +72,7 @@ class StressTestRunner { console.log('') } - private async connect(): Promise { + private async connect (): Promise { try { this.connection = await sql.promises.open(this.config.connectionString) console.log('Connected to database') @@ -82,7 +82,7 @@ class StressTestRunner { } } - private async disconnect(): Promise { + private async disconnect (): Promise { if (this.connection) { try { await this.connection.promises.close() @@ -93,7 +93,7 @@ class StressTestRunner { } } - private async setupTestTable(): Promise { + private async setupTestTable (): Promise { try { // Drop table if exists await this.connection.promises.query(` @@ -145,7 +145,7 @@ class StressTestRunner { } } - private async cleanupTestTable(): Promise { + private async cleanupTestTable (): Promise { try { await this.connection.promises.query(` IF OBJECT_ID('dbo.stress_test_table', 'U') IS NOT NULL @@ -157,7 +157,7 @@ class StressTestRunner { } } - private async runScenario(scenario: TestScenario): Promise { + private async runScenario (scenario: TestScenario): Promise { console.log(`\nRunning scenario: ${scenario.name}`) console.log(`Description: ${scenario.description}`) console.log(`Iterations: ${this.config.iterations}`) @@ -166,11 +166,11 @@ class StressTestRunner { this.startTime = Date.now() this.iterationCount = 0 this.errors = [] - + // Force GC and record baseline memory if ((global as any).gc) { (global as any).gc() - await new Promise(resolve => setTimeout(resolve, 100)) + await new Promise(resolve => { setTimeout(resolve, 100) }) } this.memoryBaseline = this.getMemoryUsage() this.logMemory('Baseline') @@ -192,7 +192,7 @@ class StressTestRunner { // Force GC periodically if ((global as any).gc && i > 0 && i % this.config.gcInterval === 0) { (global as any).gc() - await new Promise(resolve => setTimeout(resolve, 10)) + await new Promise(resolve => { setTimeout(resolve, 10) }) } } catch (err) { @@ -205,7 +205,7 @@ class StressTestRunner { } // Final report - const totalTime = (Date.now() - this.startTime!) / 1000 + const totalTime = (Date.now() - this.startTime) / 1000 const rate = this.iterationCount / totalTime console.log('\n----------------------------------------') @@ -214,7 +214,7 @@ class StressTestRunner { console.log(`Total time: ${totalTime.toFixed(2)} seconds`) console.log(`Average rate: ${rate.toFixed(1)} operations/second`) console.log(`Errors: ${this.errors.length}`) - + if (this.errors.length > 0) { console.log('\nFirst few errors:') this.errors.slice(0, 3).forEach(({ iteration, error }) => { @@ -225,12 +225,12 @@ class StressTestRunner { // Force GC and final memory report if ((global as any).gc) { (global as any).gc() - await new Promise(resolve => setTimeout(resolve, 100)) + await new Promise(resolve => { setTimeout(resolve, 100) }) } this.logMemory('Final') } - async run(scenarioName: string): Promise { + async run (scenarioName: string): Promise { try { await this.connect() await this.setupTestTable() @@ -253,7 +253,7 @@ class StressTestRunner { } // Define stress test scenarios - private scenarios: Record = { + private readonly scenarios: Record = { 'string-select': { name: 'String Column Selection', description: 'Repeatedly select string columns to check for memory leaks', @@ -330,14 +330,14 @@ class StressTestRunner { return new Promise((resolve, reject) => { const query = conn.query('SELECT * FROM stress_test_table') let rowCount = 0 - + query.on('row', (row: any) => { rowCount++ // Access the data const _ = row }) - - query.on('done', () => resolve()) + + query.on('done', () => { resolve() }) query.on('error', reject) }) } @@ -360,7 +360,7 @@ class StressTestRunner { // CLI interface if (require.main === module) { const args = process.argv.slice(2) - + if (args.length === 0) { console.log('Usage: node stress-test-runner.js [options]') console.log('\nAvailable scenarios:') @@ -389,7 +389,7 @@ if (require.main === module) { for (let i = 1; i < args.length; i += 2) { const option = args[i] const value = args[i + 1] - + switch (option) { case '--iterations': config.iterations = parseInt(value, 10) @@ -429,4 +429,4 @@ if (require.main === module) { }) } -export = StressTestRunner \ No newline at end of file +export = StressTestRunner