Skip to content

Commit 24304a6

Browse files
chore: update errors to be stringified when object (#32663)
* chore: update errors to be stringified when object * handle circular dependencies * add serialize-error npm package * downgrade serialize-error to cjs compatible version * move to prod dep * move safeErrorSerialize further out into Cloud dir * remove redundancies in studio tests * handle undefined * fix undefined
1 parent 9756ec1 commit 24304a6

File tree

7 files changed

+258
-3
lines changed

7 files changed

+258
-3
lines changed

packages/server/lib/cloud/api/studio/report_studio_error.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import Debug from 'debug'
33
import { stripPath } from '../../strip_path'
44
const debug = Debug('cypress:server:cloud:api:studio:report_studio_errors')
55
import { logError } from '@packages/stderr-filtering'
6+
import exception from '../../exception'
67

78
export interface ReportStudioErrorOptions {
89
cloudApi: StudioCloudApi
@@ -57,7 +58,10 @@ export function reportStudioError ({
5758
let errorObject: Error
5859

5960
if (!(error instanceof Error)) {
60-
errorObject = new Error(String(error))
61+
// Use safe serialization that handles circular references and other edge cases
62+
const message = exception.safeErrorSerialize(error)
63+
64+
errorObject = new Error(message)
6165
} else {
6266
errorObject = error
6367
}

packages/server/lib/cloud/exception.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,36 @@ import user from './user'
66
import system from '../util/system'
77
import { stripPath } from './strip_path'
88

9+
const { serializeError } = require('serialize-error')
10+
911
export = {
12+
/**
13+
* Safely serializes an error object to a string, handling circular references
14+
* and other non-serializable values that would cause JSON.stringify to throw.
15+
*/
16+
safeErrorSerialize (error: unknown): string {
17+
if (typeof error === 'string') {
18+
return error
19+
}
20+
21+
try {
22+
// Use serialize-error package to handle complex error objects safely
23+
const serialized = serializeError(error)
24+
25+
const result = JSON.stringify(serialized)
26+
27+
// JSON.stringify returns undefined for undefined input, but we need to return a string
28+
if (typeof result === 'undefined') {
29+
return 'undefined'
30+
}
31+
32+
return result
33+
} catch (e) {
34+
// If even serialize-error fails, use a generic fallback
35+
return `[Non-serializable object: ${error?.constructor?.name || 'Object'}]`
36+
}
37+
},
38+
1039
getErr (err: Error) {
1140
return {
1241
name: stripPath(err.name),

packages/server/lib/cloud/studio/studio.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import path from 'path'
66
import { reportStudioError, ReportStudioErrorOptions } from '../api/studio/report_studio_error'
77
import crypto, { BinaryLike } from 'crypto'
88
import { StudioElectron } from './StudioElectron'
9+
import exception from '../exception'
910

1011
interface StudioServer { default: StudioServerDefaultShape }
1112

@@ -124,7 +125,10 @@ export class StudioManager implements StudioManagerShape {
124125
let actualError: Error
125126

126127
if (!(error instanceof Error)) {
127-
actualError = new Error(String(error))
128+
// Use safe serialization that handles circular references and other edge cases
129+
const message = exception.safeErrorSerialize(error)
130+
131+
actualError = new Error(message)
128132
} else {
129133
actualError = error
130134
}
@@ -154,7 +158,10 @@ export class StudioManager implements StudioManagerShape {
154158
let actualError: Error
155159

156160
if (!(error instanceof Error)) {
157-
actualError = new Error(String(error))
161+
// Use safe serialization that handles circular references and other edge cases
162+
const message = exception.safeErrorSerialize(error)
163+
164+
actualError = new Error(message)
158165
} else {
159166
actualError = error
160167
}

packages/server/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
"sanitize-filename": "1.6.3",
119119
"semver": "^7.7.1",
120120
"send": "0.19.0",
121+
"serialize-error": "^7.0.1",
121122
"shell-env": "3.0.1",
122123
"signal-exit": "3.0.7",
123124
"squirrelly": "7.9.2",

packages/server/test/unit/cloud/api/studio/report_studio_error_spec.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,45 @@ describe('lib/cloud/api/studio/report_studio_error', () => {
149149
)
150150
})
151151

152+
it('converts non-Error objects to Error instances', () => {
153+
const objectError = {
154+
additionalData: { type: 'studio:panel:opened' },
155+
message: 'Something went wrong',
156+
code: 'TELEMETRY_ERROR',
157+
}
158+
159+
reportStudioError({
160+
cloudApi,
161+
studioHash: 'abc123',
162+
projectSlug: 'test-project',
163+
error: objectError,
164+
studioMethod: 'telemetryService',
165+
})
166+
167+
expect(cloudRequestStub).to.be.calledWithMatch(
168+
'http://localhost:1234/studio/errors',
169+
{
170+
studioHash: 'abc123',
171+
projectSlug: 'test-project',
172+
errors: [{
173+
name: 'Error',
174+
message: sinon.match.string,
175+
stack: sinon.match((stack) => stack.includes('<stripped-path>report_studio_error_spec.ts')),
176+
code: undefined,
177+
errno: undefined,
178+
studioMethod: 'telemetryService',
179+
studioMethodArgs: undefined,
180+
}],
181+
},
182+
{
183+
headers: {
184+
'Content-Type': 'application/json',
185+
'x-cypress-version': '1.2.3',
186+
},
187+
},
188+
)
189+
})
190+
152191
it('handles Error objects correctly', () => {
153192
const error = new Error('test error')
154193

packages/server/test/unit/cloud/exceptions_spec.js

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,4 +228,116 @@ at bar <stripped-path>bar.js:92\
228228
})
229229
})
230230
})
231+
232+
context('.safeErrorSerialize', () => {
233+
it('returns string as-is when error is already a string', () => {
234+
const stringError = 'Simple string error'
235+
236+
expect(exception.safeErrorSerialize(stringError)).to.eq('Simple string error')
237+
})
238+
239+
it('serializes plain objects properly', () => {
240+
const objectError = {
241+
additionalData: { type: 'studio:panel:opened' },
242+
message: 'Something went wrong',
243+
code: 'TELEMETRY_ERROR',
244+
}
245+
246+
const result = exception.safeErrorSerialize(objectError)
247+
248+
expect(result).to.eq(JSON.stringify(objectError))
249+
})
250+
251+
it('handles circular reference objects safely without throwing', () => {
252+
// Create an object with circular reference
253+
const circularError = {
254+
message: 'Circular reference error',
255+
code: 'CIRCULAR_ERROR',
256+
}
257+
258+
circularError.self = circularError // Create circular reference
259+
260+
const result = exception.safeErrorSerialize(circularError)
261+
262+
expect(result).to.eq(JSON.stringify({
263+
message: 'Circular reference error',
264+
code: 'CIRCULAR_ERROR',
265+
self: '[Circular]',
266+
}))
267+
})
268+
269+
it('handles Error objects correctly', () => {
270+
const error = new Error('test error')
271+
272+
error.code = 'TEST_CODE'
273+
error.errno = 123
274+
275+
const result = exception.safeErrorSerialize(error)
276+
277+
// serializeError should preserve Error properties
278+
const parsed = JSON.parse(result)
279+
280+
expect(parsed.message).to.eq('test error')
281+
expect(parsed.name).to.eq('Error')
282+
expect(parsed.code).to.eq('TEST_CODE')
283+
expect(parsed.errno).to.eq(123)
284+
})
285+
286+
it('handles null and undefined gracefully', () => {
287+
expect(exception.safeErrorSerialize(null)).to.eq('null')
288+
expect(exception.safeErrorSerialize(undefined)).to.eq('undefined')
289+
})
290+
291+
it('handles primitive types', () => {
292+
expect(exception.safeErrorSerialize(42)).to.eq('42')
293+
expect(exception.safeErrorSerialize(true)).to.eq('true')
294+
expect(exception.safeErrorSerialize(false)).to.eq('false')
295+
})
296+
297+
it('provides fallback for non-serializable objects', () => {
298+
// Create an object that might cause issues
299+
const problematicObject = {
300+
get value () {
301+
throw new Error('Cannot access value')
302+
},
303+
}
304+
305+
const result = exception.safeErrorSerialize(problematicObject)
306+
307+
expect(result).to.match(/^\[Non-serializable object:/)
308+
})
309+
310+
it('handles deeply nested objects', () => {
311+
const deepObject = {
312+
level1: {
313+
level2: {
314+
level3: {
315+
level4: {
316+
message: 'Deep error',
317+
data: [1, 2, 3, { nested: true }],
318+
},
319+
},
320+
},
321+
},
322+
}
323+
324+
const result = exception.safeErrorSerialize(deepObject)
325+
326+
expect(result).to.eq(JSON.stringify(deepObject))
327+
})
328+
329+
it('handles arrays with mixed content', () => {
330+
const arrayError = [
331+
'string',
332+
42,
333+
{ message: 'object in array' },
334+
null,
335+
undefined,
336+
]
337+
338+
const result = exception.safeErrorSerialize(arrayError)
339+
340+
expect(result).to.eq(JSON.stringify(arrayError))
341+
})
342+
})
231343
})

packages/server/test/unit/cloud/studio/studio_spec.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,27 @@ describe('lib/cloud/studio', () => {
7777
expect(studioManager.status).to.eq('IN_ERROR')
7878
expect(studio.reportError).to.be.calledWithMatch(error, 'initializeRoutes', {})
7979
})
80+
81+
it('handles non-Error objects by converting them to Error instances', () => {
82+
const objectError = {
83+
additionalData: { type: 'studio:panel:opened' },
84+
message: 'Something went wrong',
85+
}
86+
87+
sinon.stub(studio, 'initializeRoutes').throws(objectError)
88+
sinon.stub(studio, 'reportError')
89+
90+
studioManager.initializeRoutes({} as any)
91+
92+
expect(studioManager.status).to.eq('IN_ERROR')
93+
expect(studio.reportError).to.be.calledWithMatch(
94+
sinon.match((error) => {
95+
return error instanceof Error
96+
}),
97+
'initializeRoutes',
98+
{},
99+
)
100+
})
80101
})
81102

82103
describe('asynchronous method invocation', () => {
@@ -92,6 +113,27 @@ describe('lib/cloud/studio', () => {
92113
expect(studio.reportError).to.be.calledWithMatch(error, 'initializeStudioAI', {})
93114
})
94115

116+
it('handles non-Error objects in async methods by converting them to Error instances', async () => {
117+
const objectError = {
118+
additionalData: { type: 'studio:panel:opened' },
119+
message: 'Async error occurred',
120+
}
121+
122+
sinon.stub(studio, 'initializeStudioAI').throws(objectError)
123+
sinon.stub(studio, 'reportError')
124+
125+
await studioManager.initializeStudioAI({} as any)
126+
127+
expect(studioManager.status).to.eq('IN_ERROR')
128+
expect(studio.reportError).to.be.calledWithMatch(
129+
sinon.match((error) => {
130+
return error instanceof Error
131+
}),
132+
'initializeStudioAI',
133+
{},
134+
)
135+
})
136+
95137
it('does not set state IN_ERROR when a non-essential async method fails', async () => {
96138
const error = new Error('foo')
97139

@@ -101,6 +143,27 @@ describe('lib/cloud/studio', () => {
101143

102144
expect(studioManager.status).to.eq('ENABLED')
103145
})
146+
147+
it('handles non-Error objects in non-essential async methods without changing status', async () => {
148+
const objectError = {
149+
additionalData: { type: 'studio:panel:opened' },
150+
message: 'Non-essential error occurred',
151+
}
152+
153+
sinon.stub(studio, 'captureStudioEvent').throws(objectError)
154+
sinon.stub(studio, 'reportError')
155+
156+
await studioManager.captureStudioEvent({} as any)
157+
158+
expect(studioManager.status).to.eq('ENABLED')
159+
expect(studio.reportError).to.be.calledWithMatch(
160+
sinon.match((error) => {
161+
return error instanceof Error
162+
}),
163+
'captureStudioEvent',
164+
{},
165+
)
166+
})
104167
})
105168

106169
describe('initializeRoutes', () => {

0 commit comments

Comments
 (0)