Skip to content
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

feat!: new ReaderWatch class for handling deleted or replaced files #27

Merged
merged 26 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
681b07c
chore: add failing tests
gmaclennan Oct 17, 2024
2220448
feat!: handle file deletions and replacements in server.js
gmaclennan Oct 17, 2024
704523c
fix: wait for event loop after deletion
gmaclennan Oct 17, 2024
09e3963
fix tests - await event loop after file changes
gmaclennan Oct 17, 2024
9575d1c
temp: only run server tests in CI
gmaclennan Oct 17, 2024
7420971
feat: fix file paths for windows
gmaclennan Oct 17, 2024
830c710
add some debugging for CI tests
gmaclennan Oct 17, 2024
e8329b6
fix tests by waiting for changes to propogate
gmaclennan Oct 17, 2024
bd41a90
more CI debugging
gmaclennan Oct 17, 2024
817e052
some fixes
gmaclennan Oct 17, 2024
5d5b419
wait for longer to debug win
gmaclennan Oct 17, 2024
2a2c56f
handle file deletions on windows
gmaclennan Oct 17, 2024
9707a42
more fixes
gmaclennan Oct 17, 2024
d6462b9
fixes
gmaclennan Oct 17, 2024
6b4ee5b
fix for win
gmaclennan Oct 17, 2024
753a3aa
run all tests again
gmaclennan Oct 17, 2024
ba3e2d7
Fix error handling for win
gmaclennan Oct 17, 2024
99c9eee
cleanup
gmaclennan Oct 17, 2024
081774f
try with setImmediate again
gmaclennan Oct 17, 2024
05888ff
Merge branch 'main' into feat/watch-file-changes
gmaclennan Oct 17, 2024
388ac87
run all tests again
gmaclennan Oct 17, 2024
4c1ccc5
refactor into ReaderWatch class
gmaclennan Oct 17, 2024
ffb3438
fix breaking test
gmaclennan Oct 17, 2024
ea4cdeb
add ReaderWatch export
gmaclennan Oct 17, 2024
bdf91b1
remove unnecessary awaits when calling temporaryFile()
achou11 Oct 17, 2024
4022ec2
simplify PluginOptions type
achou11 Oct 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/** @typedef {import('./types.js').SMPStyle} SMPStyle */

export { default as Reader } from './reader.js'
export { default as ReaderWatch } from './reader-watch.js'
export { default as Writer } from './writer.js'
export { default as Server } from './server.js'
export { default as StyleDownloader } from './style-downloader.js'
Expand Down
133 changes: 133 additions & 0 deletions lib/reader-watch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import { once } from 'events'

import fs from 'node:fs'
import fsPromises from 'node:fs/promises'

import Reader from './reader.js'
import { ENOENT, isFileNotThereError } from './utils/errors.js'
import { noop } from './utils/misc.js'

/** @implements {Pick<Reader, keyof Reader>} */
export default class ReaderWatch {
achou11 marked this conversation as resolved.
Show resolved Hide resolved
/** @type {Reader | undefined} */
#reader
/** @type {Reader | undefined} */
#maybeReader
/** @type {Promise<Reader> | undefined} */
#readerOpeningPromise
#filepath
/** @type {fs.FSWatcher | undefined} */
#watch

/**
* @param {string} filepath
*/
constructor(filepath) {
this.#filepath = filepath
// Call this now to catch any synchronous errors
this.#tryToWatchFile()
// eagerly open Reader
this.#get().catch(noop)
}

#tryToWatchFile() {
if (this.#watch) return
try {
this.#watch = fs
.watch(this.#filepath, { persistent: false }, () => {
this.#reader?.close().catch(noop)
this.#reader = undefined
this.#maybeReader = undefined
this.#readerOpeningPromise = undefined
// Close the watcher (which on some platforms will continue watching
// the previous file) so on the next request we will start watching
// the new file
this.#watch?.close()
this.#watch = undefined
})
.on('error', noop)
} catch (error) {
if (isFileNotThereError(error)) {
// Ignore: File does not exist yet, but we'll try to open it later
} else {
throw error
}
}
}

async #get() {
if (isWin() && (this.#reader || this.#readerOpeningPromise)) {
// On Windows, the file watcher does not recognize file deletions, so we
// need to check if the file still exists each time
try {
await fsPromises.stat(this.#filepath)
} catch {
this.#watch?.close()
this.#watch = undefined
this.#reader?.close().catch(noop)
this.#reader = undefined
this.#maybeReader = undefined
this.#readerOpeningPromise = undefined
}
}
// Need to retry this each time in case it failed initially because the file
// was not present, or if the file was moved or deleted.
this.#tryToWatchFile()
// A lovely promise tangle to confuse future readers... sorry.
//
// 1. If the reader is already open, return it.
// 2. If the reader is in the process of opening, return a promise that will
// return the reader instance if it opened without error, or throw.
// 3. If the reader threw an error during opening, try to open it again next
// time this is called.
if (this.#reader) return this.#reader
if (this.#readerOpeningPromise) return this.#readerOpeningPromise
this.#maybeReader = new Reader(this.#filepath)
this.#readerOpeningPromise = this.#maybeReader
.opened()
.then(() => {
if (!this.#maybeReader) {
throw new ENOENT(this.#filepath)
}
this.#reader = this.#maybeReader
return this.#reader
})
.finally(() => {
this.#maybeReader = undefined
this.#readerOpeningPromise = undefined
})
return this.#readerOpeningPromise
}

/** @type {Reader['opened']} */
async opened() {
const reader = await this.#get()
return reader.opened()
}

/** @type {Reader['getStyle']} */
async getStyle(baseUrl = null) {
const reader = await this.#get()
return reader.getStyle(baseUrl)
}

/** @type {Reader['getResource']} */
async getResource(path) {
const reader = await this.#get()
return reader.getResource(path)
}

async close() {
const reader = await this.#get()
if (this.#watch) {
this.#watch.close()
await once(this.#watch, 'close')
}
await reader.close()
}
}

/** @returns {boolean} */
function isWin() {
return process.platform === 'win32'
}
83 changes: 19 additions & 64 deletions lib/server.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
import createError from 'http-errors'

import Reader from './reader.js'
import { isFileNotThereError } from './utils/errors.js'
import { noop } from './utils/misc.js'

/** @import { FastifyPluginCallback, FastifyReply } from 'fastify' */
/** @import { Resource } from './reader.js' */

/**
* @typedef {object} PluginOptions
* @property {string} [prefix]
* @typedef {object} PluginOptionsFilepath
* @property {string} filepath Path to styled map package (`.smp`) file
*/
/**
* @typedef {object} PluginOptionsReader
* @property {Pick<Reader, keyof Reader>} reader SMP Reader interface (also supports ReaderWatch)
*/
/**
* @typedef {PluginOptionsFilepath | PluginOptionsReader} PluginOptions
*/

/**
* @param {FastifyReply} reply
Expand All @@ -33,25 +40,21 @@ function sendResource(reply, resource) {
*
* @type {FastifyPluginCallback<PluginOptions>}
*/
export default function (fastify, { filepath }, done) {
const deferredReader = new DeferredReader(filepath)
export default function (fastify, opts, done) {
const reader = 'reader' in opts ? opts.reader : new Reader(opts.filepath)

fastify.addHook('onClose', async () => {
try {
const reader = await deferredReader.get()
await reader.close()
} catch {
// ignore
}
})
// Only close the reader if it was created by this plugin
if (!('reader' in opts)) {
fastify.addHook('onClose', () => reader.close().catch(noop))
}

fastify.get('/style.json', async () => {
try {
const reader = await deferredReader.get()
const baseUrl = new URL(fastify.prefix, fastify.listeningOrigin)
return reader.getStyle(baseUrl.href)
const style = await reader.getStyle(baseUrl.href)
return style
} catch (error) {
if (isENOENT(error)) {
if (isFileNotThereError(error)) {
throw createError(404, error.message)
}
throw error
Expand All @@ -63,62 +66,14 @@ export default function (fastify, { filepath }, done) {
const path = request.params['*']

try {
const reader = await deferredReader.get()
const resource = await reader.getResource(path)
return sendResource(reply, resource)
} catch (error) {
if (isENOENT(error)) {
if (isFileNotThereError(error)) {
throw createError(404, error.message)
}
throw error
}
})
done()
}

/**
* @param {unknown} error
* @returns {error is Error & { code: 'ENOENT' }}
*/
function isENOENT(error) {
return error instanceof Error && 'code' in error && error.code === 'ENOENT'
}

class DeferredReader {
/** @type {Reader | undefined} */
#reader
/** @type {Promise<Reader> | undefined} */
#readerOpeningPromise
#filepath

/**
* @param {string} filepath
*/
constructor(filepath) {
this.#filepath = filepath
this.get().catch(noop)
}

async get() {
// A lovely promise tangle to confuse future readers... sorry.
//
// 1. If the reader is already open, return it.
// 2. If the reader is in the process of opening, return a promise that will
// return the reader instance if it opened without error, or throw.
// 3. If the reader threw an error during opening, try to open it again next
// time this is called.
if (this.#reader) return this.#reader
if (this.#readerOpeningPromise) return this.#readerOpeningPromise
const maybeReader = new Reader(this.#filepath)
this.#readerOpeningPromise = maybeReader
.opened()
.then(() => {
this.#reader = maybeReader
return this.#reader
})
.finally(() => {
this.#readerOpeningPromise = undefined
})
return this.#readerOpeningPromise
}
}
15 changes: 15 additions & 0 deletions lib/utils/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,18 @@ export class ENOENT extends Error {
this.path = path
}
}

/**
* Returns true if the error if because a file is not found. On Windows, some
* operations like fs.watch() throw an EPERM error rather than ENOENT.
*
* @param {unknown} error
* @returns {error is Error & { code: 'ENOENT' | 'EPERM' }}
*/
export function isFileNotThereError(error) {
return (
error instanceof Error &&
'code' in error &&
(error.code === 'ENOENT' || error.code === 'EPERM')
)
}
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
"types": "./dist/reader.d.ts",
"import": "./lib/reader.js"
},
"./reader-watch": {
"types": "./dist/reader-watch.d.ts",
"import": "./lib/reader-watch.js"
},
"./writer": {
"types": "./dist/writer.d.ts",
"import": "./lib/writer.js"
Expand Down
Binary file added test/fixtures/osm-bright-z6.smp
Binary file not shown.
Loading
Loading