From 9ae566342376dd46ba8a8293c140b3e486e48c83 Mon Sep 17 00:00:00 2001 From: Malik Glossop Date: Fri, 22 May 2026 10:51:24 +0200 Subject: [PATCH] Merge pull request #33160 from overleaf/copilot/fix-typeerror-out-of-memory fix: normalize string errors at pdf-caching call sites before passing to OError.tag() GitOrigin-RevId: 0259de81cca72e3b9c304f68b087a627db8f1980 --- .../util/normalize-string-error.ts | 13 +++++ .../pdf-preview/util/pdf-caching-transport.ts | 2 + .../features/pdf-preview/util/pdf-caching.ts | 16 ++++-- .../util/normalize-string-error.test.ts | 54 +++++++++++++++++++ 4 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 services/web/frontend/js/features/pdf-preview/util/normalize-string-error.ts create mode 100644 services/web/test/frontend/features/pdf-preview/util/normalize-string-error.test.ts diff --git a/services/web/frontend/js/features/pdf-preview/util/normalize-string-error.ts b/services/web/frontend/js/features/pdf-preview/util/normalize-string-error.ts new file mode 100644 index 0000000000..e112c52b79 --- /dev/null +++ b/services/web/frontend/js/features/pdf-preview/util/normalize-string-error.ts @@ -0,0 +1,13 @@ +/** + * V8 can throw the bare string "out of memory" (instead of an Error) from + * buffer-allocation paths such as `new Uint8Array(N)` and + * `Response.prototype.arrayBuffer()`. Wrap such string errors in an Error so + * that downstream consumers (e.g. OError.tag, Sentry) which assume an Error + * object continue to work. + * + * Non-string values are returned unchanged, so genuine programming bugs that + * `throw null`/`throw 42`/etc. still surface unmasked. + */ +export function normalizeStringError(err: unknown): unknown { + return typeof err === 'string' ? new Error(err) : err +} diff --git a/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.ts b/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.ts index 8b096b7b80..f8c2166478 100644 --- a/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.ts +++ b/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.ts @@ -1,5 +1,6 @@ import OError from '@overleaf/o-error' import { fallbackRequest, fetchRange, preprocessFileOnce } from './pdf-caching' +import { normalizeStringError } from './normalize-string-error' import { captureException } from '@/infrastructure/error-reporter' import { EDITOR_SESSION_ID, getPdfCachingMetrics } from './metrics' import { @@ -210,6 +211,7 @@ export function generatePdfCachingTransportFactory() { fallbackToCacheURL: getOutputPDFURLFromCache(), }) .catch(err => { + err = normalizeStringError(err) if (abortSignal.aborted) return if (canTryFromCache(err)) return fetchFromCache() if (isExpectedError(err)) { diff --git a/services/web/frontend/js/features/pdf-preview/util/pdf-caching.ts b/services/web/frontend/js/features/pdf-preview/util/pdf-caching.ts index dbc5828e46..5ceb630498 100644 --- a/services/web/frontend/js/features/pdf-preview/util/pdf-caching.ts +++ b/services/web/frontend/js/features/pdf-preview/util/pdf-caching.ts @@ -6,6 +6,7 @@ import { ProcessedPDFFile, } from '@ol-types/compile' import OError from '@overleaf/o-error' +import { normalizeStringError } from './normalize-string-error' import { PdfCachingMetricsFull } from './types' const PDF_JS_CHUNK_SIZE = 128 * 1024 @@ -523,7 +524,11 @@ export async function fallbackRequest({ checkChunkResponse(response, end - start, init) return await response.arrayBuffer() } catch (e) { - throw OError.tag(e, 'fallback request failed', { url, start, end }) + throw OError.tag(normalizeStringError(e), 'fallback request failed', { + url, + start, + end, + }) } } @@ -650,7 +655,8 @@ async function fetchChunk({ delete init.signal // omit the signal from the cache cachedUrls.set(chunk.hash, { url, init }) } - } catch (err1) { + } catch (rawErr1) { + const err1 = normalizeStringError(rawErr1) if ('hash' in chunk && chunk.hash) { cachedUrls.delete(chunk.hash) } @@ -1046,7 +1052,11 @@ export async function fetchRange({ metrics, }) } catch (err) { - throw OError.tag(err, 'cannot fetch chunk', { chunk, url, init }) + throw OError.tag(normalizeStringError(err), 'cannot fetch chunk', { + chunk, + url, + init, + }) } finally { timer.finishBlockingCompute() } diff --git a/services/web/test/frontend/features/pdf-preview/util/normalize-string-error.test.ts b/services/web/test/frontend/features/pdf-preview/util/normalize-string-error.test.ts new file mode 100644 index 0000000000..08cc5ecd5b --- /dev/null +++ b/services/web/test/frontend/features/pdf-preview/util/normalize-string-error.test.ts @@ -0,0 +1,54 @@ +import { expect } from 'chai' +import OError from '@overleaf/o-error' +import { normalizeStringError } from '@/features/pdf-preview/util/normalize-string-error' + +describe('normalizeStringError', function () { + it('wraps a string in an Error so OError.tag/Sentry can consume it', function () { + // V8 throws the bare string "out of memory" (instead of an Error) from + // some buffer-allocation paths (e.g. `new Uint8Array(N)`, + // `Response.prototype.arrayBuffer()`). + const result = normalizeStringError('out of memory') + expect(result).to.be.an.instanceOf(Error) + expect((result as Error).message).to.equal('out of memory') + expect((result as Error).stack).to.be.a('string') + }) + + it('returns an Error instance unchanged', function () { + const original = new Error('boom') + expect(normalizeStringError(original)).to.equal(original) + }) + + it('returns a custom Error subclass unchanged', function () { + class CustomError extends Error {} + const original = new CustomError('boom') + expect(normalizeStringError(original)).to.equal(original) + }) + + it('returns non-string non-Error values unchanged so genuine bugs surface', function () { + // The helper deliberately does not wrap `null`/`undefined`/numbers/etc., + // so code paths that `throw null` or `throw 42` continue to surface as + // bugs rather than being masked. + expect(normalizeStringError(null)).to.equal(null) + expect(normalizeStringError(undefined)).to.equal(undefined) + expect(normalizeStringError(42)).to.equal(42) + const obj = { foo: 'bar' } + expect(normalizeStringError(obj)).to.equal(obj) + }) + + it('produces an Error that OError.tag can attach metadata to', function () { + // Round-trip the realistic usage: tag a normalised string error with + // some info, and verify both the tag and the info make it through. + const err = OError.tag( + normalizeStringError('out of memory'), + 'fallback request failed', + { url: '/project/abc/output.pdf', start: 0, end: 1024 } + ) + expect(err).to.be.an.instanceOf(Error) + expect((err as Error).message).to.equal('out of memory') + expect(OError.getFullInfo(err)).to.deep.include({ + url: '/project/abc/output.pdf', + start: 0, + end: 1024, + }) + }) +})