diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 97994cc..4d5f783 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -15,7 +15,7 @@ env: jobs: build: runs-on: ubuntu-latest - needs: test + needs: [test, frontend-test] permissions: contents: read packages: write @@ -198,3 +198,20 @@ jobs: docker run --rm \ -v "${GITHUB_WORKSPACE}:/src:ro" \ legacy-tests:${{ matrix.lancedb }} + + frontend-test: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v5 + + - name: Set up Node.js + uses: actions/setup-node@v6 + with: + node-version: '24' + + - name: Check frontend syntax + run: node --check web/vanilla/app.js + + - name: Run frontend tests + run: node --test web/vanilla/tests/*.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index ae58bbf..f5f4a3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Frontend content is now built with safe DOM APIs instead of `innerHTML`, preventing dataset-controlled column names from injecting markup through vector tooltips (#32). +- Older dataset, metadata, and row responses no longer overwrite a newer selection when requests finish out of order (#79). - `docker build` no longer fails with "the destination must be a directory and end with a /". `COPY backend/*.py .` needs a trailing slash when it copies more than one file. The classic builder rejected it, the BuildKit builder did not (#62). - CI publishes no image until the test job passes. The build job now depends on the test job, so a failing test stops the release (#66). - CI and release builds preserve the legacy LanceDB 0.3.1, 0.3.4, and 0.5 variants by reusing their last published dependency layers. Their packages are no longer available from PyPI (#92). diff --git a/web/vanilla/app.js b/web/vanilla/app.js index 70edb12..76f0370 100644 --- a/web/vanilla/app.js +++ b/web/vanilla/app.js @@ -10,6 +10,9 @@ class LanceViewer { this.dataPathConfigured = false; this.currentDataLocation = ''; this.currentReference = 'main'; + this.datasetListRequestId = 0; + this.metadataRequestId = 0; + this.dataRequestId = 0; this.initializeElements(); this.setupEventListeners(); @@ -124,6 +127,8 @@ class LanceViewer { this.currentReference = this.elements.datasetReference.value.trim() || 'main'; this.elements.datasetReference.value = this.currentReference; this.elements.connectionError.textContent = ''; + this.metadataRequestId++; + this.dataRequestId++; this.currentDataset = null; this.elements.datasetHeader.style.display = 'none'; this.elements.columnSection.style.display = 'none'; @@ -190,6 +195,8 @@ class LanceViewer { } async loadDatasets() { + const requestId = ++this.datasetListRequestId; + try { this.replaceWithMessage(this.elements.datasetList, 'Loading datasets...', 'loading'); const params = this.contextParams(false); @@ -200,11 +207,13 @@ class LanceViewer { } const data = await response.json(); + if (requestId !== this.datasetListRequestId) return false; + this.elements.datasetList.replaceChildren(); if (data.datasets.length === 0) { this.replaceWithMessage(this.elements.datasetList, 'No datasets found', 'loading'); - return; + return true; } data.datasets.forEach(dataset => { @@ -214,9 +223,13 @@ class LanceViewer { item.addEventListener('click', () => this.selectDataset(dataset, item)); this.elements.datasetList.appendChild(item); }); + return true; } catch (error) { + if (requestId !== this.datasetListRequestId) return false; + this.replaceWithMessage(this.elements.datasetList, 'Failed to load datasets', 'error'); this.showConnectionError(error.message); + return false; } } @@ -242,19 +255,33 @@ class LanceViewer { } async loadMetadata() { + const requestId = ++this.metadataRequestId; + const datasetName = this.currentDataset; + try { const params = this.contextParams(); const response = await fetch( - `${this.apiBase}/datasets/${encodeURIComponent(this.currentDataset)}/metadata?${params}` + `${this.apiBase}/datasets/${encodeURIComponent(datasetName)}/metadata?${params}` ); if (!response.ok) { throw new Error(await this.responseError(response)); } const metadata = await response.json(); + + if ( + requestId !== this.metadataRequestId + || datasetName !== this.currentDataset + ) return false; + this.renderSchema(metadata.fields); this.renderColumns(metadata.columns); return true; } catch (error) { + if ( + requestId !== this.metadataRequestId + || datasetName !== this.currentDataset + ) return false; + this.showConnectionError(error.message); this.showError(error.message); return false; @@ -328,6 +355,8 @@ class LanceViewer { async loadData() { if (!this.currentDataset) return; + const requestId = ++this.dataRequestId; + const datasetName = this.currentDataset; this.showLoading(); try { @@ -343,13 +372,18 @@ class LanceViewer { } const response = await fetch( - `${this.apiBase}/datasets/${encodeURIComponent(this.currentDataset)}/rows?${params}` + `${this.apiBase}/datasets/${encodeURIComponent(datasetName)}/rows?${params}` ); if (!response.ok) { throw new Error(await this.responseError(response)); } const data = await response.json(); + if ( + requestId !== this.dataRequestId + || datasetName !== this.currentDataset + ) return false; + this.totalRows = data.total; this.renderTable(data.rows); this.updatePagination(); @@ -357,6 +391,11 @@ class LanceViewer { return true; } catch (error) { + if ( + requestId !== this.dataRequestId + || datasetName !== this.currentDataset + ) return false; + this.hideLoading(); this.showConnectionError(error.message); this.showError(error.message); diff --git a/web/vanilla/tests/stale-responses.test.js b/web/vanilla/tests/stale-responses.test.js new file mode 100644 index 0000000..8069899 --- /dev/null +++ b/web/vanilla/tests/stale-responses.test.js @@ -0,0 +1,238 @@ +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const test = require('node:test'); +const vm = require('node:vm'); + + +function deferred() { + let resolve; + let reject; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return {promise, resolve, reject}; +} + + +function loadViewer(fetchImpl) { + const sourcePath = path.join(__dirname, '..', 'app.js'); + const source = `${fs.readFileSync(sourcePath, 'utf8')}\n` + + 'globalThis.TestLanceViewer = LanceViewer;'; + const sandbox = { + console, + document: {addEventListener() {}}, + fetch: fetchImpl, + URLSearchParams, + }; + vm.createContext(sandbox); + vm.runInContext(source, sandbox, {filename: sourcePath}); + return sandbox.TestLanceViewer; +} + + +function createViewer(Viewer) { + const viewer = Object.create(Viewer.prototype); + Object.assign(viewer, { + apiBase: 'http://viewer.test', + allColumns: [], + currentDataset: 'first', + currentPage: 0, + dataRequestId: 0, + datasetListRequestId: 0, + metadataRequestId: 0, + pageSize: 50, + selectedColumns: [], + totalRows: 0, + }); + viewer.contextParams = () => new URLSearchParams(); + viewer.showLoading = () => {}; + return viewer; +} + + +test('a stale dataset-list response cannot replace a newer connection', async () => { + const firstResponse = deferred(); + const secondResponse = deferred(); + const responses = [firstResponse.promise, secondResponse.promise]; + const Viewer = loadViewer(() => responses.shift()); + const viewer = createViewer(Viewer); + const messages = []; + const appended = []; + const errors = []; + + viewer.elements = { + datasetList: { + appendChild: item => appended.push(item), + replaceChildren() {}, + }, + }; + viewer.replaceWithMessage = (_element, message) => messages.push(message); + viewer.showConnectionError = message => errors.push(message); + + const firstRequest = viewer.loadDatasets(); + const secondRequest = viewer.loadDatasets(); + + secondResponse.resolve({ + ok: true, + json: async () => ({datasets: []}), + }); + assert.equal(await secondRequest, true); + + firstResponse.resolve({ + ok: true, + json: async () => ({datasets: ['stale']}), + }); + assert.equal(await firstRequest, false); + + assert.deepEqual(messages, [ + 'Loading datasets...', + 'Loading datasets...', + 'No datasets found', + ]); + assert.deepEqual(appended, []); + assert.deepEqual(errors, []); +}); + + +test('a stale row response cannot replace a newer dataset', async () => { + const firstResponse = deferred(); + const secondResponse = deferred(); + const responses = [firstResponse.promise, secondResponse.promise]; + const Viewer = loadViewer(() => responses.shift()); + const viewer = createViewer(Viewer); + const rendered = []; + const errors = []; + let hiddenCount = 0; + + viewer.renderTable = rows => rendered.push(rows); + viewer.updatePagination = () => {}; + viewer.hideLoading = () => { hiddenCount++; }; + viewer.showConnectionError = message => errors.push(message); + viewer.showError = message => errors.push(message); + + const firstRequest = viewer.loadData(); + viewer.currentDataset = 'second'; + const secondRequest = viewer.loadData(); + + secondResponse.resolve({ + ok: true, + json: async () => ({rows: [{id: 'second'}], total: 1}), + }); + await secondRequest; + + firstResponse.resolve({ + ok: true, + json: async () => ({rows: [{id: 'first'}], total: 99}), + }); + await firstRequest; + + assert.deepEqual(rendered, [[{id: 'second'}]]); + assert.equal(viewer.totalRows, 1); + assert.equal(hiddenCount, 1); + assert.deepEqual(errors, []); +}); + + +test('a stale row response cannot undo a newer column selection', async () => { + const firstResponse = deferred(); + const secondResponse = deferred(); + const responses = [firstResponse.promise, secondResponse.promise]; + const Viewer = loadViewer(() => responses.shift()); + const viewer = createViewer(Viewer); + const rendered = []; + + viewer.renderTable = rows => rendered.push(rows); + viewer.updatePagination = () => {}; + viewer.hideLoading = () => {}; + viewer.showConnectionError = () => {}; + viewer.showError = () => {}; + + const allColumnsRequest = viewer.loadData(); + viewer.allColumns = [{name: 'id'}, {name: 'text'}]; + viewer.selectedColumns = ['id']; + const selectedColumnsRequest = viewer.loadData(); + + secondResponse.resolve({ + ok: true, + json: async () => ({rows: [{id: 1}], total: 1}), + }); + await selectedColumnsRequest; + + firstResponse.resolve({ + ok: true, + json: async () => ({rows: [{id: 1, text: 'stale'}], total: 1}), + }); + await allColumnsRequest; + + assert.deepEqual(rendered, [[{id: 1}]]); +}); + + +test('a stale metadata response cannot replace a newer dataset', async () => { + const firstResponse = deferred(); + const secondResponse = deferred(); + const responses = [firstResponse.promise, secondResponse.promise]; + const Viewer = loadViewer(() => responses.shift()); + const viewer = createViewer(Viewer); + const renderedSchemas = []; + const renderedColumns = []; + const errors = []; + + viewer.renderSchema = fields => renderedSchemas.push(fields); + viewer.renderColumns = columns => renderedColumns.push(columns); + viewer.showConnectionError = message => errors.push(message); + viewer.showError = message => errors.push(message); + + const firstRequest = viewer.loadMetadata(); + viewer.currentDataset = 'second'; + const secondRequest = viewer.loadMetadata(); + + secondResponse.resolve({ + ok: true, + json: async () => ({fields: [{name: 'second'}], columns: [{name: 'second'}]}), + }); + await secondRequest; + + firstResponse.resolve({ + ok: true, + json: async () => ({fields: [{name: 'first'}], columns: [{name: 'first'}]}), + }); + await firstRequest; + + assert.deepEqual(renderedSchemas, [[{name: 'second'}]]); + assert.deepEqual(renderedColumns, [[{name: 'second'}]]); + assert.deepEqual(errors, []); +}); + + +test('a stale failed row request cannot replace the current error state', async () => { + const firstResponse = deferred(); + const secondResponse = deferred(); + const responses = [firstResponse.promise, secondResponse.promise]; + const Viewer = loadViewer(() => responses.shift()); + const viewer = createViewer(Viewer); + const errors = []; + + viewer.renderTable = () => {}; + viewer.updatePagination = () => {}; + viewer.hideLoading = () => {}; + viewer.showConnectionError = message => errors.push(message); + viewer.showError = message => errors.push(message); + + const firstRequest = viewer.loadData(); + viewer.currentDataset = 'second'; + const secondRequest = viewer.loadData(); + + secondResponse.resolve({ + ok: true, + json: async () => ({rows: [], total: 0}), + }); + await secondRequest; + + firstResponse.reject(new Error('old request failed')); + await firstRequest; + + assert.deepEqual(errors, []); +});