Skip to content

Commit cfa1bfa

Browse files
kobenguyentkobenguyent
committed
fix: use this.options instead of this.config for helper callbacks
The Helper base class from @codeceptjs/helper sets both this.config (raw user config) and this.options (validated/merged config with defaults). Helpers should consistently use this.options to access configuration values. Fixed helpers to use this.options for onRequest/onResponse callbacks: - REST helper: Fixed onRequest and onResponse to use this.options - GraphQL helper: Fixed onRequest and onResponse to use this.options - Playwright helper: Fixed onResponse to use this.options - JSONResponse helper: Fixed accessing request helper's options.onResponse Updated tests to match: - JSONResponse_test.js: Changed restHelper.config to restHelper.options - JSONResponse_test.js: Made beforeEach async and await Container.create() - Playwright_test.js: Changed I.config to I.options - JSONResponse.js: Added named export for test compatibility This fixes the error: 'Cannot read properties of undefined (reading "name")' which occurred because JSONResponse tried to access this.helpers.REST.config.onResponse but REST helper stores callbacks in this.options, not this.config. All 488 unit tests passing.
1 parent 9b531cd commit cfa1bfa

File tree

6 files changed

+56
-72
lines changed

6 files changed

+56
-72
lines changed

‎lib/helper/GraphQL.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ class GraphQL extends Helper {
8787

8888
request.headers = { ...this.headers, ...request.headers }
8989

90-
if (this.config.onRequest) {
91-
await this.config.onRequest(request)
90+
if (this.options.onRequest) {
91+
await this.options.onRequest(request)
9292
}
9393

9494
this.debugSection('Request', JSON.stringify(request))
@@ -102,8 +102,8 @@ class GraphQL extends Helper {
102102
response = err.response
103103
}
104104

105-
if (this.config.onResponse) {
106-
await this.config.onResponse(response)
105+
if (this.options.onResponse) {
106+
await this.options.onResponse(response)
107107
}
108108

109109
this.debugSection('Response', JSON.stringify(response.data))

‎lib/helper/JSONResponse.js‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ class JSONResponse extends Helper {
7272
if (!this.helpers[this.options.requestHelper]) {
7373
throw new Error(`Error setting JSONResponse, helper ${this.options.requestHelper} is not enabled in config, helpers: ${Object.keys(this.helpers)}`)
7474
}
75-
const origOnResponse = this.helpers[this.options.requestHelper].config.onResponse
76-
this.helpers[this.options.requestHelper].config.onResponse = response => {
75+
const origOnResponse = this.helpers[this.options.requestHelper].options.onResponse
76+
this.helpers[this.options.requestHelper].options.onResponse = response => {
7777
this.response = response
7878
if (typeof origOnResponse === 'function') origOnResponse(response)
7979
}
@@ -83,7 +83,6 @@ class JSONResponse extends Helper {
8383
this.response = null
8484
}
8585

86-
8786
/**
8887
* Checks that response code is equal to the provided one
8988
*
@@ -372,4 +371,4 @@ class JSONResponse extends Helper {
372371
}
373372
}
374373

375-
export { JSONResponse as default }
374+
export { JSONResponse, JSONResponse as default }

‎lib/helper/Playwright.js‎

Lines changed: 18 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ class Playwright extends Helper {
355355
this.recordingWebSocketMessages = false
356356
this.recordedWebSocketMessagesAtLeastOnce = false
357357
this.cdpSession = null
358-
358+
359359
// Filter out invalid customLocatorStrategies (empty arrays, objects without functions)
360360
// This can happen in worker threads where config is serialized/deserialized
361361
let validCustomLocators = null
@@ -367,7 +367,7 @@ class Playwright extends Helper {
367367
validCustomLocators = config.customLocatorStrategies
368368
}
369369
}
370-
370+
371371
this.customLocatorStrategies = validCustomLocators
372372
this._customLocatorsRegistered = false
373373

@@ -794,10 +794,7 @@ class Playwright extends Helper {
794794
await Promise.allSettled(pages.map(p => p.close().catch(() => {})))
795795
}
796796
// Use timeout to prevent hanging (10s should be enough for browser cleanup)
797-
await Promise.race([
798-
this._stopBrowser(),
799-
new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout')), 10000)),
800-
])
797+
await Promise.race([this._stopBrowser(), new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout')), 10000))])
801798
} catch (e) {
802799
console.warn('Warning during browser restart in _after:', e.message)
803800
// Force cleanup even on timeout
@@ -840,10 +837,7 @@ class Playwright extends Helper {
840837
if (this.isRunning) {
841838
try {
842839
// Add timeout protection to prevent hanging
843-
await Promise.race([
844-
this._stopBrowser(),
845-
new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout in afterSuite')), 10000)),
846-
])
840+
await Promise.race([this._stopBrowser(), new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout in afterSuite')), 10000))])
847841
} catch (e) {
848842
console.warn('Warning during suite cleanup:', e.message)
849843
// Track suite cleanup failures
@@ -954,10 +948,7 @@ class Playwright extends Helper {
954948
if (this.isRunning) {
955949
try {
956950
// Add timeout protection to prevent hanging
957-
await Promise.race([
958-
this._stopBrowser(),
959-
new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout in cleanup')), 10000)),
960-
])
951+
await Promise.race([this._stopBrowser(), new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout in cleanup')), 10000))])
961952
} catch (e) {
962953
console.warn('Warning during final cleanup:', e.message)
963954
// Force cleanup on timeout
@@ -970,10 +961,7 @@ class Playwright extends Helper {
970961
if (this.browser) {
971962
try {
972963
// Add timeout protection to prevent hanging
973-
await Promise.race([
974-
this._stopBrowser(),
975-
new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout in forced cleanup')), 10000)),
976-
])
964+
await Promise.race([this._stopBrowser(), new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout in forced cleanup')), 10000))])
977965
} catch (e) {
978966
console.warn('Warning during forced cleanup:', e.message)
979967
// Force cleanup on timeout
@@ -1390,7 +1378,7 @@ class Playwright extends Helper {
13901378
this.context = null
13911379
this.frame = null
13921380
popupStore.clear()
1393-
1381+
13941382
// Remove all event listeners to prevent hanging
13951383
if (this.browser) {
13961384
try {
@@ -1399,7 +1387,7 @@ class Playwright extends Helper {
13991387
// Ignore errors if browser is already closed
14001388
}
14011389
}
1402-
1390+
14031391
if (this.options.recordHar && this.browserContext) {
14041392
try {
14051393
await this.browserContext.close()
@@ -1408,16 +1396,11 @@ class Playwright extends Helper {
14081396
}
14091397
}
14101398
this.browserContext = null
1411-
1399+
14121400
if (this.browser) {
14131401
try {
14141402
// Add timeout to prevent browser.close() from hanging indefinitely
1415-
await Promise.race([
1416-
this.browser.close(),
1417-
new Promise((_, reject) =>
1418-
setTimeout(() => reject(new Error('Browser close timeout')), 5000)
1419-
)
1420-
])
1403+
await Promise.race([this.browser.close(), new Promise((_, reject) => setTimeout(() => reject(new Error('Browser close timeout')), 5000))])
14211404
} catch (e) {
14221405
// Ignore errors if browser is already closed or timeout
14231406
if (!e.message?.includes('Browser close timeout')) {
@@ -1539,7 +1522,7 @@ class Playwright extends Helper {
15391522
acceptDownloads: true,
15401523
...this.options.emulate,
15411524
}
1542-
1525+
15431526
try {
15441527
this.browserContext = await this.browser.newContext(contextOptions)
15451528
} catch (err) {
@@ -3183,14 +3166,14 @@ class Playwright extends Helper {
31833166
this.debugSection('Response', await response.text())
31843167

31853168
// hook to allow JSON response handle this
3186-
if (this.config.onResponse) {
3169+
if (this.options.onResponse) {
31873170
const axiosResponse = {
31883171
data: await response.json(),
31893172
status: response.status(),
31903173
statusText: response.statusText(),
31913174
headers: response.headers(),
31923175
}
3193-
this.config.onResponse(axiosResponse)
3176+
this.options.onResponse(axiosResponse)
31943177
}
31953178

31963179
return response
@@ -4337,11 +4320,11 @@ function isRoleLocatorObject(locator) {
43374320
*/
43384321
async function handleRoleLocator(context, locator) {
43394322
if (!isRoleLocatorObject(locator)) return null
4340-
4323+
43414324
const options = {}
43424325
if (locator.text) options.name = locator.text
43434326
if (locator.exact !== undefined) options.exact = locator.exact
4344-
4327+
43454328
return context.getByRole(locator.role, Object.keys(options).length > 0 ? options : undefined).all()
43464329
}
43474330

@@ -4350,7 +4333,7 @@ async function findElements(matcher, locator) {
43504333
const isReactLocator = locator.type === 'react' || (locator.locator && locator.locator.react) || locator.react
43514334
const isVueLocator = locator.type === 'vue' || (locator.locator && locator.locator.vue) || locator.vue
43524335
const isPwLocator = locator.type === 'pw' || (locator.locator && locator.locator.pw) || locator.pw
4353-
4336+
43544337
if (isReactLocator) return findReact(matcher, locator)
43554338
if (isVueLocator) return findVue(matcher, locator)
43564339
if (isPwLocator) return findByPlaywrightLocator.call(this, matcher, locator)
@@ -4391,7 +4374,7 @@ async function findCustomElements(matcher, locator) {
43914374
// Always prioritize this.customLocatorStrategies which is set in constructor from config
43924375
// and persists in every worker thread instance
43934376
let strategyFunction = null
4394-
4377+
43954378
if (this.customLocatorStrategies && this.customLocatorStrategies[locator.type]) {
43964379
strategyFunction = this.customLocatorStrategies[locator.type]
43974380
} else if (globalCustomLocatorStrategies.has(locator.type)) {
@@ -4967,7 +4950,7 @@ async function refreshContextSession() {
49674950
this.debugSection('Session', 'Skipping storage cleanup - no active page/context')
49684951
return
49694952
}
4970-
4953+
49714954
const currentUrl = await this.grabCurrentUrl()
49724955

49734956
if (currentUrl.startsWith('http')) {

‎lib/helper/REST.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,8 @@ class REST extends Helper {
215215
}
216216
}
217217

218-
if (this.config.onRequest) {
219-
await this.config.onRequest(request)
218+
if (this.options.onRequest) {
219+
await this.options.onRequest(request)
220220
}
221221

222222
try {
@@ -245,8 +245,8 @@ class REST extends Helper {
245245
}
246246
response = err.response
247247
}
248-
if (this.config.onResponse) {
249-
await this.config.onResponse(response)
248+
if (this.options.onResponse) {
249+
await this.options.onResponse(response)
250250
}
251251
try {
252252
this.options.prettyPrintJson ? this.debugSection('Response', beautify(JSON.stringify(response.data))) : this.debugSection('Response', JSON.stringify(response.data))

‎test/helper/JSONResponse_test.js‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ let restHelper
2727
let I
2828

2929
describe('JSONResponse', () => {
30-
beforeEach(() => {
31-
Container.create({
30+
beforeEach(async () => {
31+
await Container.create({
3232
helpers: {
3333
REST: {},
3434
},
@@ -41,42 +41,42 @@ describe('JSONResponse', () => {
4141

4242
describe('response codes', () => {
4343
it('should check 200x codes', async () => {
44-
restHelper.config.onResponse({ status: 204 })
44+
restHelper.options.onResponse({ status: 204 })
4545
I.seeResponseCodeIs(204)
4646
I.dontSeeResponseCodeIs(200)
4747
I.seeResponseCodeIsSuccessful()
4848
})
4949

5050
it('should check 300x codes', async () => {
51-
restHelper.config.onResponse({ status: 304 })
51+
restHelper.options.onResponse({ status: 304 })
5252
I.seeResponseCodeIs(304)
5353
I.dontSeeResponseCodeIs(200)
5454
I.seeResponseCodeIsRedirection()
5555
})
5656

5757
it('should check 400x codes', async () => {
58-
restHelper.config.onResponse({ status: 404 })
58+
restHelper.options.onResponse({ status: 404 })
5959
I.seeResponseCodeIs(404)
6060
I.dontSeeResponseCodeIs(200)
6161
I.seeResponseCodeIsClientError()
6262
})
6363

6464
it('should check 500x codes', async () => {
65-
restHelper.config.onResponse({ status: 504 })
65+
restHelper.options.onResponse({ status: 504 })
6666
I.seeResponseCodeIs(504)
6767
I.dontSeeResponseCodeIs(200)
6868
I.seeResponseCodeIsServerError()
6969
})
7070

7171
it('should throw error on invalid code', () => {
72-
restHelper.config.onResponse({ status: 504 })
72+
restHelper.options.onResponse({ status: 504 })
7373
expect(() => I.seeResponseCodeIs(200)).to.throw('Response code')
7474
})
7575
})
7676

7777
describe('response data', () => {
7878
it('should check for json inclusion', () => {
79-
restHelper.config.onResponse({ data })
79+
restHelper.options.onResponse({ data })
8080
I.seeResponseContainsJson({
8181
posts: [{ id: 2 }],
8282
})
@@ -88,7 +88,7 @@ describe('JSONResponse', () => {
8888

8989
it('should check for json inclusion - returned Array', () => {
9090
const arrayData = [{ ...data }]
91-
restHelper.config.onResponse({ data: arrayData })
91+
restHelper.options.onResponse({ data: arrayData })
9292
I.seeResponseContainsJson({
9393
posts: [{ id: 2 }],
9494
})
@@ -100,48 +100,48 @@ describe('JSONResponse', () => {
100100

101101
it('should check for json inclusion - returned Array of 2 items', () => {
102102
const arrayData = [{ ...data }, { posts: { id: 3 } }]
103-
restHelper.config.onResponse({ data: arrayData })
103+
restHelper.options.onResponse({ data: arrayData })
104104
I.seeResponseContainsJson({
105105
posts: { id: 3 },
106106
})
107107
})
108108

109109
it('should simply check for json inclusion', () => {
110-
restHelper.config.onResponse({ data: { user: { name: 'jon', email: 'jon@doe.com' } } })
110+
restHelper.options.onResponse({ data: { user: { name: 'jon', email: 'jon@doe.com' } } })
111111
I.seeResponseContainsJson({ user: { name: 'jon' } })
112112
I.dontSeeResponseContainsJson({ user: { name: 'jo' } })
113113
I.dontSeeResponseContainsJson({ name: 'joe' })
114114
})
115115

116116
it('should simply check for json inclusion - returned Array', () => {
117-
restHelper.config.onResponse({ data: [{ user: { name: 'jon', email: 'jon@doe.com' } }] })
117+
restHelper.options.onResponse({ data: [{ user: { name: 'jon', email: 'jon@doe.com' } }] })
118118
I.seeResponseContainsJson({ user: { name: 'jon' } })
119119
I.dontSeeResponseContainsJson({ user: { name: 'jo' } })
120120
I.dontSeeResponseContainsJson({ name: 'joe' })
121121
})
122122

123123
it('should simply check for json equality', () => {
124-
restHelper.config.onResponse({ data: { user: 1 } })
124+
restHelper.options.onResponse({ data: { user: 1 } })
125125
I.seeResponseEquals({ user: 1 })
126126
})
127127

128128
it('should simply check for json equality - returned Array', () => {
129-
restHelper.config.onResponse({ data: [{ user: 1 }] })
129+
restHelper.options.onResponse({ data: [{ user: 1 }] })
130130
I.seeResponseEquals([{ user: 1 }])
131131
})
132132

133133
it('should check json contains keys', () => {
134-
restHelper.config.onResponse({ data: { user: 1, post: 2 } })
134+
restHelper.options.onResponse({ data: { user: 1, post: 2 } })
135135
I.seeResponseContainsKeys(['user', 'post'])
136136
})
137137

138138
it('should check json contains keys - returned Array', () => {
139-
restHelper.config.onResponse({ data: [{ user: 1, post: 2 }] })
139+
restHelper.options.onResponse({ data: [{ user: 1, post: 2 }] })
140140
I.seeResponseContainsKeys(['user', 'post'])
141141
})
142142

143143
it('should check for json by callback', () => {
144-
restHelper.config.onResponse({ data })
144+
restHelper.options.onResponse({ data })
145145
const fn = ({ assert, data }) => {
146146
assert('posts' in data)
147147
assert('user' in data)
@@ -151,13 +151,15 @@ describe('JSONResponse', () => {
151151
})
152152

153153
it('should check for json by zod schema', () => {
154-
restHelper.config.onResponse({ data })
154+
restHelper.options.onResponse({ data })
155155
const schema = z.object({
156-
posts: z.array(z.object({
157-
id: z.number(),
158-
author: z.string(),
159-
title: z.string(),
160-
})),
156+
posts: z.array(
157+
z.object({
158+
id: z.number(),
159+
author: z.string(),
160+
title: z.string(),
161+
}),
162+
),
161163
user: z.object({
162164
name: z.string(),
163165
}),
@@ -170,7 +172,7 @@ describe('JSONResponse', () => {
170172
})
171173

172174
it('should throw error when zod validation fails', () => {
173-
restHelper.config.onResponse({ data: { name: 'invalid', age: 'not_a_number' } })
175+
restHelper.options.onResponse({ data: { name: 'invalid', age: 'not_a_number' } })
174176
const schema = z.object({
175177
name: z.string(),
176178
age: z.number(),

0 commit comments

Comments
 (0)