diff --git a/src/main/content-api-client.js b/src/main/content-api-client.js index b42b445..732cd08 100644 --- a/src/main/content-api-client.js +++ b/src/main/content-api-client.js @@ -31,7 +31,11 @@ import { buildDaLiveSourceUrl, LIST_CONTINUATION_HEADER, } from './da-live-api.js'; -import { buildFetchFailureError, buildHttpError } from './http-request-error.js'; +import { + buildFetchFailureError, + buildHttpError, + HttpRequestError, +} from './http-request-error.js'; const LIST_MAX_PAGES = 50000; @@ -48,6 +52,10 @@ function withNetworkErrorDetail(fetchImpl) { try { return await fetchImpl(url, init); } catch (err) { + // Already enriched by the injected fetch (e.g. with proxy context). + if (err instanceof HttpRequestError) { + throw err; + } throw buildFetchFailureError(init?.method || 'GET', String(url), err); } }; diff --git a/src/main/http-request-error.js b/src/main/http-request-error.js index 9195ebb..2c9dc21 100644 --- a/src/main/http-request-error.js +++ b/src/main/http-request-error.js @@ -96,14 +96,18 @@ export function describeErrorChain(err) { /** * Builds the error for a fetch that failed before any HTTP response arrived * (DNS, TLS, proxy, offline), naming the request and the underlying cause. + * `networkContext` (connectivity, resolved proxy, versions) is appended so a + * screenshot of the on-screen error is enough to diagnose proxy/VPN issues. * * @param {string} method * @param {string} url * @param {unknown} err + * @param {string} [networkContext] * @returns {HttpRequestError} */ -export function buildFetchFailureError(method, url, err) { - const message = `Network request failed: ${method} ${url} — ${describeErrorChain(err)}. ` +export function buildFetchFailureError(method, url, err, networkContext = '') { + const contextPart = networkContext ? ` [${networkContext}]` : ''; + const message = `Network request failed: ${method} ${url} — ${describeErrorChain(err)}${contextPart}. ` + 'Check your network, VPN, or proxy connection.'; return new HttpRequestError(message, { method, url }); } diff --git a/src/main/index.js b/src/main/index.js index c541e3d..73c4a66 100644 --- a/src/main/index.js +++ b/src/main/index.js @@ -26,7 +26,7 @@ import { isDaUnauthorizedError, } from './content-api-shared.js'; import { ContentApiClient } from './content-api-client.js'; -import { HttpRequestError } from './http-request-error.js'; +import { buildFetchFailureError, HttpRequestError } from './http-request-error.js'; import { DA_TOKEN_FILENAME, getAuthStatus, getValidToken, loadStoredToken, } from './da-auth.js'; @@ -288,9 +288,46 @@ async function handleDaUnauthorized(err, site, backend) { return new Error(detail); } +/** + * Gathers network-stack context for a failed request: connectivity, the + * proxy Chromium resolved for this URL, and versions. Appended to the + * on-screen error and the log so a user's screenshot or log file is enough + * to tell DNS, proxy, VPN, and TLS problems apart. + * + * @param {string} url + * @returns {Promise} + */ +async function describeNetworkContext(url) { + const parts = []; + try { + parts.push(net.isOnline() ? 'online' : 'no network connection'); + } catch { + parts.push('connectivity unknown'); + } + try { + const proxy = await session.defaultSession.resolveProxy(url); + parts.push(`proxy: ${proxy || 'unknown'}`); + } catch (err) { + parts.push(`proxy lookup failed: ${err.message}`); + } + parts.push(`app ${app.getVersion()}, electron ${process.versions.electron}`); + return parts.join('; '); +} + // Chromium network stack (not Node's undici): honors the OS proxy/PAC // configuration and certificate store, which corporate networks require. -const chromiumFetch = (url, init) => net.fetch(url, init); +// Failures are logged and enriched with network context before rethrowing. +const chromiumFetch = async (url, init) => { + try { + return await net.fetch(url, init); + } catch (err) { + const method = init?.method || 'GET'; + const context = await describeNetworkContext(String(url)); + const failure = buildFetchFailureError(method, String(url), err, context); + log.scope('net').error(failure.message); + throw failure; + } +}; async function withContentClient(site, fn) { const backend = site.apiBackend || API_BACKEND_DA_LIVE; diff --git a/src/main/preview-server.js b/src/main/preview-server.js index d4160e2..e3866bb 100644 --- a/src/main/preview-server.js +++ b/src/main/preview-server.js @@ -49,8 +49,27 @@ const SKIP_REQUEST_HEADERS = new Set([ // EDS rejects that foreign origin with a 403, so strip it — the proxy stands in // for the .aem.page origin and presents requests as same-origin. 'origin', + // Subresource requests carry the localhost proxy page as Referer. Chromium's + // network stack (net.fetch) kills requests whose Referer doesn't match the + // destination with net::ERR_BLOCKED_BY_CLIENT — and it leaks the local proxy + // origin upstream — so never forward it. + 'referer', ]); +/** + * Never forward a header the browser manages itself. Besides the fixed skip + * list, sec-fetch-* describes the webview's fetch context, not the proxy's: + * Chromium rejects a net.fetch carrying `sec-fetch-mode: cors` with + * net::ERR_INVALID_ARGUMENT (module scripts break, stylesheets survive) and + * sets its own sec-fetch-* on the upstream request anyway. + * + * @param {string} name lower-cased header name + * @returns {boolean} + */ +function shouldSkipRequestHeader(name) { + return SKIP_REQUEST_HEADERS.has(name) || name.startsWith('sec-fetch-'); +} + const SKIP_RESPONSE_HEADERS = new Set([ 'connection', 'content-encoding', @@ -111,7 +130,17 @@ function createAuthenticatedFetch(siteToken, fetchFn = fetch) { * @param {string|null|undefined} siteToken * @param {typeof fetch} fetchFn */ -async function proxyUpstream(req, res, upstreamUrl, proxyHost, siteToken, fetchFn) { +/** + * @param {import('node:http').IncomingMessage} req + * @param {import('node:http').ServerResponse} res + * @param {string} upstreamUrl + * @param {string} proxyHost + * @param {string|null|undefined} siteToken + * @param {typeof fetch} fetchFn + * @param {ReturnType} scope + * @returns {Promise} the upstream response (headers already sent) + */ +async function proxyUpstream(req, res, upstreamUrl, proxyHost, siteToken, fetchFn, scope) { /** @type {Record} */ const reqHeaders = { 'x-forwarded-host': proxyHost, @@ -120,7 +149,7 @@ async function proxyUpstream(req, res, upstreamUrl, proxyHost, siteToken, fetchF const names = Object.keys(req.headers); for (let i = 0; i < names.length; i += 1) { const name = names[i]; - if (!SKIP_REQUEST_HEADERS.has(name.toLowerCase())) { + if (!shouldSkipRequestHeader(name.toLowerCase())) { reqHeaders[name] = /** @type {string} */ (req.headers[name]); } } @@ -147,17 +176,18 @@ async function proxyUpstream(req, res, upstreamUrl, proxyHost, siteToken, fetchF res.writeHead(upstream.status, respHeaders); - if (req.method === 'HEAD') { + if (req.method === 'HEAD' || !upstream.body) { res.end(); - return; + return upstream; } - if (!upstream.body) { - res.end(); - return; - } - - Readable.fromWeb(upstream.body).pipe(res); + const bodyStream = Readable.fromWeb(upstream.body); + bodyStream.on('error', (err) => { + scope.error(`upstream body stream failed for ${upstreamUrl}: ${err.message}`); + res.destroy(err); + }); + bodyStream.pipe(res); + return upstream; } /** @@ -187,16 +217,18 @@ function upstreamStatusNeedsAuth(status, siteToken) { * @param {string} upstreamUrl * @param {string|null|undefined} siteToken * @param {typeof fetch} fetchFn + * @param {ReturnType} scope * @returns {Promise} */ -async function upstreamRequiresAuth(upstreamUrl, siteToken, fetchFn) { +async function upstreamRequiresAuth(upstreamUrl, siteToken, fetchFn, scope) { if (siteToken) { return false; } try { const resp = await fetchFn(upstreamUrl, { method: 'HEAD', redirect: 'follow' }); return upstreamStatusNeedsAuth(resp.status, siteToken); - } catch { + } catch (err) { + scope.warn(`auth probe HEAD ${upstreamUrl} failed: ${err.message}`); return false; } } @@ -266,21 +298,30 @@ export async function startPreviewServer(deps) { const fetchFn = deps.fetchFn || fetch; const server = createServer(async (req, res) => { + const startedAt = Date.now(); + const elapsed = () => `${Date.now() - startedAt}ms`; + if (!req.url) { + scope.warn('400 — request without URL'); sendText(res, 400, 'Bad request'); return; } if (req.method !== 'GET' && req.method !== 'HEAD') { + scope.warn(`405 — ${req.method} ${req.url}`); sendText(res, 405, 'Method not allowed'); return; } + // One line per request start so hung requests are visible in the log. + scope.info(`→ ${req.method} ${req.url} [dest: ${req.headers['sec-fetch-dest'] || '?'}]`); + const requestUrl = new URL(req.url, 'http://127.0.0.1'); const previewPath = pathnameToPreviewPath(requestUrl.pathname); const site = await deps.getActiveSite(); if (!site) { + scope.warn(`503 — no active site for ${req.method} ${req.url} (site switched or not activated?)`); sendText(res, 503, 'No active site for preview'); return; } @@ -293,6 +334,7 @@ export async function startPreviewServer(deps) { ); const token = deps.getToken ? await deps.getToken(site) : null; + const tokenNote = token ? 'site token' : 'no site token'; const authFetch = createAuthenticatedFetch(token, fetchFn); const syncFolder = await deps.getSyncFolder(); @@ -301,7 +343,7 @@ export async function startPreviewServer(deps) { const localFile = await resolveLocalContentFile(localRoot, previewPath); const authBlocked = localFile && isMainFrameDocumentRequest(req) - && await upstreamRequiresAuth(upstreamUrl, token, fetchFn); + && await upstreamRequiresAuth(upstreamUrl, token, fetchFn, scope); if (localFile && !authBlocked) { try { const headHtml = await headHtmlCache.resolve({ @@ -326,23 +368,32 @@ export async function startPreviewServer(deps) { 'content-type': contentType, 'access-control-allow-origin': '*', }, req.method === 'HEAD' ? '' : body); - scope.info(`local ${localFile.relativePath} -> 200 ${pathWithQuery}`); + scope.info(`local ${localFile.relativePath} -> 200 ${pathWithQuery} (${contentType}, ${elapsed()})`); return; } catch (err) { - scope.warn(`failed to read local file ${localFile.filePath}: ${err.message}`); + scope.warn(`failed to render local file ${localFile.filePath}: ${err.message} — falling back to upstream`); } } else if (authBlocked) { scope.info(`local ${localFile.relativePath} blocked — upstream requires auth`); } + } else { + scope.debug(`no sync folder configured — proxying ${pathWithQuery} upstream`); } const proxyHost = req.headers.host || requestUrl.host; try { - await proxyUpstream(req, res, upstreamUrl, proxyHost, token, fetchFn); - scope.info(`proxy ${pathWithQuery} -> ${upstreamUrl} (${res.statusCode})`); + const upstream = await proxyUpstream(req, res, upstreamUrl, proxyHost, token, fetchFn, scope); + const contentType = upstream.headers.get('content-type') || 'no content-type'; + const logLine = `proxy ${pathWithQuery} -> ${upstreamUrl} ` + + `(${upstream.status}, ${contentType}, ${tokenNote}, ${elapsed()})`; + if (upstream.status >= 400) { + scope.warn(logLine); + } else { + scope.info(logLine); + } maybeNotifyAuthRequired(site, req, res.statusCode, token, deps.onAuthRequired); } catch (err) { - scope.error(`proxy failed for ${upstreamUrl}: ${err.message}`); + scope.error(`proxy failed for ${upstreamUrl} (${tokenNote}, ${elapsed()}): ${err.message}`); if (!res.headersSent) { sendText(res, 502, `Failed to proxy AEM request: ${err.message}`); } diff --git a/test/content-api-client.test.js b/test/content-api-client.test.js index 1da3146..5b98697 100644 --- a/test/content-api-client.test.js +++ b/test/content-api-client.test.js @@ -18,6 +18,7 @@ import { } from '../src/main/content-api-shared.js'; import { ContentApiClient } from '../src/main/content-api-client.js'; import { + buildFetchFailureError, composeHttpErrorMessage, describeErrorChain, HttpRequestError, @@ -73,6 +74,45 @@ test('network failures name the request and underlying cause', async () => { ); }); +test('buildFetchFailureError appends network context when provided', () => { + const err = buildFetchFailureError( + 'GET', + 'https://admin.da.live/list/org/site/', + new Error('net::ERR_PROXY_CONNECTION_FAILED'), + 'online; proxy: PROXY proxy.example.com:8080; app 1.6.2, electron 33.0.0', + ); + + assert.match(err.message, /net::ERR_PROXY_CONNECTION_FAILED/); + assert.match(err.message, /\[online; proxy: PROXY proxy\.example\.com:8080; app 1\.6\.2, electron 33\.0\.0\]/); + assert.match(err.message, /network, VPN, or proxy/); +}); + +test('pre-enriched network errors from the injected fetch are not re-wrapped', async () => { + const enriched = buildFetchFailureError( + 'GET', + 'https://admin.da.live/list/org/site/', + new Error('net::ERR_NAME_NOT_RESOLVED'), + 'no network connection; proxy: DIRECT', + ); + const fetchImpl = async () => { + throw enriched; + }; + const client = new ContentApiClient('token', API_BACKEND_DA_LIVE, fetchImpl); + + await assert.rejects( + () => client.list('org', 'site', '/'), + (err) => { + assert.equal(err, enriched); + assert.equal( + (err.message.match(/Network request failed/g) || []).length, + 1, + 'context must not be wrapped twice', + ); + return true; + }, + ); +}); + test('401 errors keep the unauthorized prefix and carry request detail', async () => { const fetchImpl = async () => new Response('token rejected', { status: 401, diff --git a/test/preview-server.test.js b/test/preview-server.test.js index 76da7c1..66c3628 100644 --- a/test/preview-server.test.js +++ b/test/preview-server.test.js @@ -188,6 +188,72 @@ test('preview server strips localhost Origin before upstream fetch', async () => } }); +test('preview server strips localhost Referer before upstream fetch', async () => { + let forwardedReferer; + const site = { + org: 'org', + repo: 'id', + previewUrl: 'https://main--id--org.aem.page', + }; + const server = await startPreviewServer({ + getActiveSite: async () => site, + getSyncFolder: async () => null, + getToken: async () => null, + fetchFn: async (_url, init) => { + forwardedReferer = init?.headers?.referer; + return new Response('ok', { status: 200 }); + }, + }); + + try { + // Chromium's net.fetch rejects a Referer that doesn't match the upstream + // destination with net::ERR_BLOCKED_BY_CLIENT, so it must never forward. + await fetch(`${server.baseUrl}/styles/styles.css`, { + headers: { Referer: 'http://127.0.0.1:9999/some/page' }, + }); + assert.equal(forwardedReferer, undefined); + } finally { + await server.close(); + } +}); + +test('preview server strips sec-fetch-* headers before upstream fetch', async () => { + /** @type {Record|undefined} */ + let forwardedHeaders; + const site = { + org: 'org', + repo: 'id', + previewUrl: 'https://main--id--org.aem.page', + }; + const server = await startPreviewServer({ + getActiveSite: async () => site, + getSyncFolder: async () => null, + getToken: async () => null, + fetchFn: async (_url, init) => { + forwardedHeaders = init?.headers; + return new Response('ok', { status: 200 }); + }, + }); + + try { + // Chromium's net.fetch rejects a forwarded `sec-fetch-mode: cors` with + // net::ERR_INVALID_ARGUMENT, so the webview's fetch metadata must stay out. + await fetch(`${server.baseUrl}/scripts/aem.js`, { + headers: { + 'Sec-Fetch-Dest': 'script', + 'Sec-Fetch-Mode': 'cors', + 'Sec-Fetch-Site': 'same-origin', + Accept: '*/*', + }, + }); + const forwardedNames = Object.keys(forwardedHeaders || {}).map((n) => n.toLowerCase()); + assert.ok(!forwardedNames.some((n) => n.startsWith('sec-fetch-'))); + assert.ok(forwardedNames.includes('accept')); + } finally { + await server.close(); + } +}); + test('preview server proxies to the site previewUrl origin (aem.page not aem.live)', async () => { let upstreamUrl; const site = {