Skip to content

🐛 Fixed post card assets going missing until restart (in-memory serving) #29128

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.

Sign up for GitHub

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

Draft
sagzy wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

🐛 Fixed post card assets going missing until restart (in-memory serving) #29128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
90 changes: 81 additions & 9 deletions ghost/core/core/frontend/services/assets-minification/assets-minification-base.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,50 @@
const errors = require('@tryghost/errors');
const logging = require('@tryghost/logging');

// Minimum time between builds triggered by requests. A build that keeps
// failing (or that never produces the requested file) must not re-run for
// every incoming request. invalidate() resets the clock so config changes
// (e.g. theme activation) always rebuild immediately.
const BUILD_MIN_INTERVAL_MS = 10000;

module.exports = class AssetsMinificationBase {
minifier;

ready = false;
loading = null;

/**
* Incremented on invalidate() so an in-flight build can detect that its
* config went stale mid-build and re-run with the new config.
* @private
*/
generation = 0;

/**
* Timestamp (ms) of when the last build settled — used for retry backoff.
* @private
*/
lastBuildSettledAt = 0;

/**
* The error from the last failed build, if any.
* @private
*/
lastBuildError = null;

constructor(options = {}) {
this.options = options;
}

invalidate() {
this.generation += 1;
this.ready = false;
this.loading = null;
// Deliberately do NOT clear this.loading here: an in-flight build must
// not be forgotten, otherwise a new build could start while the old one
// is still running and two minifier runs would execute concurrently.
// ensureLoaded() re-runs load() when the generation changed mid-build.
this.lastBuildSettledAt = 0;
this.lastBuildError = null;
}

generateGlobs() {
Expand All @@ -28,6 +59,48 @@ module.exports = class AssetsMinificationBase {
});
}

/**
* Ensure the assets have been built.
*
* - Joins an in-flight build rather than starting a second one
* - Applies a retry backoff so a persistently failing (or non-producing)
* build doesn't run once per request
* - Re-runs the build when invalidate() was called mid-build, so the
* result always reflects the latest config
*
* @returns {Promise<void>} resolves when a build has settled — rejects
* with the build error when the build failed (or recently failed and
* we're inside the backoff window)
*/
ensureLoaded() {
if (this.loading) {
return this.loading;
}

if (this.lastBuildSettledAt && Date.now() - this.lastBuildSettledAt < BUILD_MIN_INTERVAL_MS) {
return this.lastBuildError ? Promise.reject(this.lastBuildError) : Promise.resolve();
}

const pending = (async () => {
let generation;
do {
generation = this.generation;
await this.load();
} while (generation !== this.generation);
this.lastBuildError = null;
})().catch((error) => {
this.lastBuildError = error;
throw error;
}).finally(() => {
this.lastBuildSettledAt = Date.now();
if (this.loading === pending) {
this.loading = null;
}
});
this.loading = pending;
return pending;
}

async minify(globs, options) {
try {
const result = await this.minifier.minify(globs, options);
Expand All @@ -52,15 +125,14 @@ module.exports = class AssetsMinificationBase {
*/
return async function serveMiddleware(req, res, next) {
if (!self.ready) {
if (!self.loading) {
const pending = self.load().finally(() => {
if (self.loading === pending) {
self.loading = null;
}
});
self.loading = pending;
try {
await self.ensureLoaded();
} catch (error) {
// A failed build must not block the request — the rejection
// would escape the async handler and hang the request under
// Express 4.
logging.error(error);
}
await self.loading;
}

next();
Expand Down
37 changes: 36 additions & 1 deletion ghost/core/core/frontend/services/assets-minification/card-assets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const debug = require('@tryghost/debug')('card-assets');
const _ = require('lodash');
const path = require('path');
const logging = require('@tryghost/logging');
const config = require('../../../shared/config');
const Minifier = require('./minifier');
const AssetsMinificationBase = require('./assets-minification-base');
Expand All @@ -18,6 +19,13 @@ module.exports = class CardAssets extends AssetsMinificationBase {
}

this.files = [];

/**
* Minified card assets, keyed by destination file name
* (e.g. `{'cards.min.css': '...'}`) — the source of truth for serving.
* @type {Object<string, string>}
*/
this.outputs = {};
}

/**
Expand Down Expand Up @@ -62,6 +70,16 @@ module.exports = class CardAssets extends AssetsMinificationBase {
return Object.keys(this.generateGlobs()).indexOf(`cards.min.${type}`) > -1;
}

/**
* Get the minified contents of a built card asset
*
* @param {string} filename e.g. 'cards.min.css'
* @returns {string|null} the minified contents, or null when not built/produced
*/
getContent(filename) {
return this.outputs[filename] ?? null;
}

invalidate(cardAssetConfig) {
if (cardAssetConfig) {
this.config = cardAssetConfig;
Expand All @@ -86,6 +104,23 @@ module.exports = class CardAssets extends AssetsMinificationBase {

debug('globs', globs);

this.files = await this.minify(globs) || [];
// Build in memory — this is the source of truth for serving, so a
// missing or unwritable content folder can never take the assets
// offline. An in-memory build cannot fail with EACCES/ENOENT on the
// destination, so a successful build is always servable.
this.outputs = await this.minifier.minifyInMemory(globs);
this.files = Object.keys(this.outputs);
this.ready = true;

// Best-effort disk write for back-compat with setups that expect the
// built files in content/public — failures are logged and must never
// affect serving.
for (const [dest, contents] of Object.entries(this.outputs)) {
try {
await this.minifier.writeFile(contents, dest);
} catch (error) {
logging.warn(`Ghost was not able to write card asset ${dest} to disk — serving it from memory. Reason: ${error.message}`);
}
}
}
};
79 changes: 58 additions & 21 deletions ghost/core/core/frontend/services/assets-minification/minifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,39 @@ class Minifier {
}
}

/**
* Run the minification pipeline for a single destination:
* glob + concat the source files, apply replacements, minify.
*
* @private
* @param {string} src source glob
* @param {string} dest destination file name (must end in .css or .js)
* @param {Object} [options]
* @param {Object} [options.replacements] Key value pairs that should get replaced in the content before minifying
* @returns {Promise<string|null>} Minified contents, or null when there was no output
*/
async minifyDestination(src, dest, options) {
let contents = await this.getSrcFileContents(src);

if (options?.replacements) {
for (const key of Object.keys(options.replacements)) {
contents = contents.replace(key, options.replacements[key]);
}
}

if (dest.endsWith('.css')) {
return await this.minifyCSS(contents);
} else if (dest.endsWith('.js')) {
return await this.minifyJS(contents);
}

throw new errors.IncorrectUsageError({
message: tpl(messages.badDestination.message, {dest}),
context: tpl(messages.badDestination.context),
help: tpl(messages.globalHelp)
});
}

/**
* Minify files
*
Expand All @@ -155,27 +188,7 @@ class Minifier {
const minifiedFiles = [];

for (const dest of destinations) {
const src = globs[dest];
let contents = await this.getSrcFileContents(src);

if (options?.replacements) {
for (const key of Object.keys(options.replacements)) {
contents = contents.replace(key, options.replacements[key]);
}
}
let minifiedContents;

if (dest.endsWith('.css')) {
minifiedContents = await this.minifyCSS(contents);
} else if (dest.endsWith('.js')) {
minifiedContents = await this.minifyJS(contents);
} else {
throw new errors.IncorrectUsageError({
message: tpl(messages.badDestination.message, {dest}),
context: tpl(messages.badDestination.context),
help: tpl(messages.globalHelp)
});
}
const minifiedContents = await this.minifyDestination(globs[dest], dest, options);

const result = await this.writeFile(minifiedContents, dest);
if (result) {
Expand All @@ -186,6 +199,30 @@ class Minifier {
debug('End');
return minifiedFiles;
}

/**
* Minify files in memory, without writing anything to disk
*
* @param {Object} globs Same shape as minify()
* @param {Object} [options]
* @param {Object} [options.replacements] Key value pairs that should get replaced in the content before minifying
* @returns {Promise<Object<string, string>>} Map of destination file name to minified contents
* (destinations whose minified output was empty are omitted)
*/
async minifyInMemory(globs, options) {
debug('Begin (in memory)', globs);
const outputs = {};

for (const dest of Object.keys(globs)) {
const minifiedContents = await this.minifyDestination(globs[dest], dest, options);
if (minifiedContents) {
outputs[dest] = minifiedContents;
}
}

debug('End (in memory)');
return outputs;
}
}

module.exports = Minifier;
72 changes: 69 additions & 3 deletions ghost/core/core/frontend/web/routers/serve-public-file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const crypto = require('crypto');
const fs = require('fs-extra');
const path = require('path');
const errors = require('@tryghost/errors');
const logging = require('@tryghost/logging');
const config = require('../../../shared/config');
const urlUtils = require('../../../shared/url-utils');
const tpl = require('@tryghost/tpl');
Expand Down Expand Up @@ -102,6 +103,70 @@ function createPublicFileMiddleware(location, file, mime, maxAge, options = {})
};
}

/**
* Serve an asset straight from an in-memory assets service (e.g. card assets)
* instead of reading it back from disk. The service is the source of truth:
* a wiped or unwritable content folder can never 404 these assets.
*
* @param {Object} service An assets-minification service exposing `ready`,
* `ensureLoaded()` and `getContent(filename)`
* @param {string} filename e.g. 'cards.min.css'
* @param {string} mime
* @param {number} maxAge
*/
function createInMemoryAssetMiddleware(service, filename, mime, maxAge) {
let cache = null;
const blogRegex = /(\{\{blog-url\}\})/g;

return async function serveInMemoryAssetMiddleware(req, res, next) {
if (!service.ready) {
try {
await service.ensureLoaded();
} catch (error) {
// A failed build must not block the request — the rejection
// would escape the async handler and hang the request under
// Express 4. If there's no content we 404 below; the service
// retries the build on a later request (past its backoff).
logging.error(error);
}
}

const content = service.getContent(filename);

if (content === null || content === undefined) {
return next(new errors.NotFoundError({
message: tpl(messages.fileNotFound),
code: 'PUBLIC_FILE_NOT_FOUND'
}));
}

// Memoize the computed body + headers keyed by the content reference,
// so the md5 isn't recomputed per request. A rebuild produces a new
// string reference, which invalidates the memo.
if (!cache || cache.content !== content) {
let str = content;

if (mime === 'text/xsl' || mime === 'text/plain' || mime === 'application/javascript') {
str = str.replace(blogRegex, urlUtils.urlFor('home', true).replace(/\/$/, ''));
}

cache = {
content,
headers: {
'Content-Type': mime,
'Content-Length': Buffer.from(str).length,
ETag: `"${crypto.createHash('md5').update(str, 'utf8').digest('hex')}"`,
'Cache-Control': `public, max-age=${maxAge}`
},
body: str
};
}

res.writeHead(200, cache.headers);
res.end(cache.body);
};
}

// Handles requests to robots.txt and favicon.ico (and caches them)
function servePublicFile(location, file, type, maxAge, options = {}) {
const publicFileMiddleware = createPublicFileMiddleware(location, file, type, maxAge, options);
Expand All @@ -127,9 +192,9 @@ function servePublicFiles(siteApp) {
// Traffic analytics tracking script
siteApp.get('/public/ghost-stats.min.js', createPublicFileMiddleware('static', 'public/ghost-stats.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge')));

// Card assets (built on the fly)
siteApp.get('/public/cards.min.css', cardAssets.serveMiddleware(), createPublicFileMiddleware('built', 'public/cards.min.css', 'text/css', config.get('caching:publicAssets:maxAge')));
siteApp.get('/public/cards.min.js', cardAssets.serveMiddleware(), createPublicFileMiddleware('built', 'public/cards.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge')));
// Card assets (built on the fly, served from memory)
siteApp.get('/public/cards.min.css', createInMemoryAssetMiddleware(cardAssets, 'cards.min.css', 'text/css', config.get('caching:publicAssets:maxAge')));
siteApp.get('/public/cards.min.js', createInMemoryAssetMiddleware(cardAssets, 'cards.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge')));

// Comment counts
siteApp.get('/public/comment-counts.min.js', createPublicFileMiddleware('static', 'public/comment-counts.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge')));
Expand Down Expand Up @@ -163,3 +228,4 @@ function servePublicFiles(siteApp) {
module.exports = servePublicFiles;
module.exports.servePublicFile = servePublicFile;
module.exports.createPublicFileMiddleware = createPublicFileMiddleware;
module.exports.createInMemoryAssetMiddleware = createInMemoryAssetMiddleware;
Loading
Loading