Просмотр исходного кода

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
Malik Glossop 2 месяцев назад
Родитель
Сommit
9ae5663423

+ 13 - 0
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
+}

+ 2 - 0
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)) {

+ 13 - 3
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()
       }

+ 54 - 0
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,
+    })
+  })
+})