Skip to content

Commit 298be55

Browse files
DAreRodzDAreRodz
authored
Merge pull request #5 from woocommerce/fix-captured-errors-in-with-scope-generators
Fix captured errors in `withScope` generators
2 parents 301d304 + c6b8847 commit 298be55

File tree

5 files changed

+283
-11
lines changed

5 files changed

+283
-11
lines changed

‎packages/e2e-tests/plugins/interactive-blocks/generator-scope/render.php

+3
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,7 @@
1313
<input readonly data-wp-bind--value="context.result" data-testid="result" />
1414
<button type="button" data-wp-on--click="callbacks.resolve" data-testid="resolve">Async resolve</button>
1515
<button type="button" data-wp-on--click="callbacks.reject" data-testid="reject">Async reject</button>
16+
<button type="button" data-wp-on--click="callbacks.capture" data-testid="capture">Async capture</button>
17+
<button type="button" data-wp-on--click="callbacks.captureThrow" data-testid="captureThrow">Async captureThrow</button>
18+
<button type="button" data-wp-on--click="callbacks.captureReturnReject" data-testid="captureReturnReject">Async captureReturnReject</button>
1619
</div>

‎packages/e2e-tests/plugins/interactive-blocks/generator-scope/view.js

+31-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44
import { store, getContext } from '@wordpress/interactivity';
55

6-
store( 'test/generator-scope', {
6+
const { callbacks } = store( 'test/generator-scope', {
77
callbacks: {
88
*resolve() {
99
try {
@@ -19,5 +19,35 @@ store( 'test/generator-scope', {
1919
getContext().result = err.toString();
2020
}
2121
},
22+
*capture() {
23+
let value = yield Promise.resolve( 1 );
24+
try {
25+
value = yield Promise.reject( 2 );
26+
} catch ( e ) {
27+
value = yield Promise.resolve( 3 );
28+
}
29+
getContext().result = value;
30+
},
31+
*throw() {
32+
const value = yield Promise.resolve( '🤯' );
33+
throw new Error( value );
34+
},
35+
*captureThrow() {
36+
try {
37+
yield callbacks.throw();
38+
} catch ( err ) {
39+
getContext().result = err.toString();
40+
}
41+
},
42+
*returnReject() {
43+
return Promise.reject( new Error( '🔚' ) );
44+
},
45+
*captureReturnReject() {
46+
try {
47+
yield callbacks.returnReject();
48+
} catch ( err ) {
49+
getContext().result = err.toString();
50+
}
51+
},
2252
},
2353
} );

‎packages/interactivity/src/test/utils.ts

+208-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
/**
22
* Internal dependencies
33
*/
4-
import { kebabToCamelCase } from '../utils';
4+
import { kebabToCamelCase, withScope } from '../utils';
5+
import { setScope, getScope, resetScope, type Scope } from '../scopes';
6+
import { setNamespace, getNamespace, resetNamespace } from '../namespaces';
57

68
describe( 'Interactivity API', () => {
79
describe( 'kebabToCamelCase', () => {
@@ -25,4 +27,209 @@ describe( 'Interactivity API', () => {
2527
expect( kebabToCamelCase( '-my-item-' ) ).toBe( 'myItem' );
2628
} );
2729
} );
30+
31+
describe( 'withScope', () => {
32+
const previousNamespace = 'previousNamespace';
33+
const previousScope = {
34+
evaluate: () => {},
35+
context: {},
36+
serverContext: {},
37+
ref: { current: null },
38+
attributes: {},
39+
};
40+
41+
const dummyNamespace = 'testNamespace';
42+
const dummyScope = {
43+
evaluate: () => {},
44+
context: { test: 'normal' },
45+
serverContext: { test: 'normal' },
46+
ref: { current: null },
47+
attributes: {},
48+
};
49+
50+
// Helper to mimic the wrapper used in state-proxy.ts and store-proxy.ts.
51+
const dummyScopeAndNS = ( callback: () => any ) => {
52+
setScope( dummyScope );
53+
setNamespace( dummyNamespace );
54+
try {
55+
return callback();
56+
} finally {
57+
resetNamespace();
58+
resetScope();
59+
}
60+
};
61+
62+
beforeEach( () => {
63+
setScope( previousScope );
64+
setNamespace( previousNamespace );
65+
} );
66+
67+
afterEach( () => {
68+
resetNamespace();
69+
resetScope();
70+
} );
71+
72+
it( 'should return a function when passed a normal function', () => {
73+
function normalFn( x: number ) {
74+
return x + 1;
75+
}
76+
const wrapped = withScope( normalFn );
77+
expect( typeof wrapped ).toBe( 'function' );
78+
} );
79+
80+
it( 'should call the original function, set the scope and namespace and restore them afterwards', () => {
81+
let called = false;
82+
let scope: Scope;
83+
let namespace: string;
84+
function normalFn( x: number ) {
85+
called = true;
86+
scope = getScope();
87+
namespace = getNamespace();
88+
return x * 2;
89+
}
90+
const wrapped = dummyScopeAndNS( () => withScope( normalFn ) );
91+
const result = wrapped( 5 );
92+
93+
expect( result ).toBe( 10 );
94+
expect( called ).toBe( true );
95+
expect( scope! ).toBe( dummyScope );
96+
expect( namespace! ).toBe( dummyNamespace );
97+
// After invocation, scope and namespace are reset.
98+
expect( getScope() ).toBe( previousScope );
99+
expect( getNamespace() ).toBe( previousNamespace );
100+
} );
101+
102+
it( 'should return an async function when passed a generator function', async () => {
103+
function* gen() {
104+
yield Promise.resolve( 'value' );
105+
return 'done';
106+
}
107+
const wrapped = dummyScopeAndNS( () => withScope( gen ) );
108+
const resultPromise = wrapped();
109+
110+
expect( resultPromise ).toBeInstanceOf( Promise );
111+
112+
const result = await resultPromise;
113+
114+
expect( result ).toBe( 'done' );
115+
// After invocation, scope and namespace are reset.
116+
expect( getScope() ).toBe( previousScope );
117+
expect( getNamespace() ).toBe( previousNamespace );
118+
} );
119+
120+
it( 'should execute a generator function step by step and yield the correct values, maintaining scope and namespace', async () => {
121+
const steps: Array< { scope: any; namespace: string } > = [];
122+
function* gen() {
123+
steps.push( { scope: getScope(), namespace: getNamespace() } );
124+
const a = yield Promise.resolve( 1 );
125+
steps.push( { scope: getScope(), namespace: getNamespace() } );
126+
const b = yield Promise.resolve( a + 1 );
127+
steps.push( { scope: getScope(), namespace: getNamespace() } );
128+
return b * 2;
129+
}
130+
const callback = dummyScopeAndNS( () => withScope( gen ) );
131+
const result = await callback();
132+
133+
expect( result ).toBe( 4 );
134+
steps.forEach( ( step ) => {
135+
expect( step.scope ).toBe( dummyScope );
136+
expect( step.namespace ).toBe( dummyNamespace );
137+
} );
138+
expect( getScope() ).toBe( previousScope );
139+
expect( getNamespace() ).toBe( previousNamespace );
140+
} );
141+
142+
it( 'should return the resolved value when a promise is returned in generator functions', async () => {
143+
function* gen() {
144+
const a = yield Promise.resolve( 3 );
145+
return Promise.resolve( a + 2 );
146+
}
147+
const wrapped = dummyScopeAndNS( () => withScope( gen ) );
148+
const result = await wrapped();
149+
expect( result ).toBe( 5 );
150+
expect( getScope() ).toBe( previousScope );
151+
expect( getNamespace() ).toBe( previousNamespace );
152+
} );
153+
154+
it( 'should accept arguments in generator functions like in normal functions', async () => {
155+
function* gen( ...values: number[] ) {
156+
let result = 0;
157+
for ( const value of values ) {
158+
result += yield Promise.resolve( value );
159+
}
160+
return result;
161+
}
162+
const wrapped = dummyScopeAndNS( () => withScope( gen ) );
163+
const result = await wrapped( 1, 2, 3, 4 );
164+
expect( result ).toBe( 10 );
165+
expect( getScope() ).toBe( previousScope );
166+
expect( getNamespace() ).toBe( previousNamespace );
167+
} );
168+
169+
it( 'should propagate errors thrown inside a generator function', async () => {
170+
function* gen() {
171+
throw new Error( 'Test Error' );
172+
}
173+
const wrapped = dummyScopeAndNS( () => withScope( gen ) );
174+
await expect( wrapped() ).rejects.toThrow( 'Test Error' );
175+
expect( getScope() ).toBe( previousScope );
176+
expect( getNamespace() ).toBe( previousNamespace );
177+
} );
178+
179+
it( 'should propagate errors when a generator function returns a rejected promise', async () => {
180+
function* gen() {
181+
return Promise.reject( new Error( 'Test Error' ) );
182+
}
183+
const wrapped = dummyScopeAndNS( () => withScope( gen ) );
184+
await expect( wrapped() ).rejects.toThrow( 'Test Error' );
185+
expect( getScope() ).toBe( previousScope );
186+
expect( getNamespace() ).toBe( previousNamespace );
187+
} );
188+
189+
it( 'hould handle captured errors within generator execution and resume correctly, maintaining scope and namespace', async () => {
190+
const steps: Array< { scope: any; namespace: string } > = [];
191+
function* gen() {
192+
let a: number;
193+
try {
194+
steps.push( {
195+
scope: getScope(),
196+
namespace: getNamespace(),
197+
} );
198+
a = yield Promise.reject( new Error( 'CatchMe' ) );
199+
} catch ( e ) {
200+
steps.push( {
201+
scope: getScope(),
202+
namespace: getNamespace(),
203+
} );
204+
a = 10;
205+
}
206+
steps.push( { scope: getScope(), namespace: getNamespace() } );
207+
const b = yield Promise.resolve( a + 5 );
208+
steps.push( { scope: getScope(), namespace: getNamespace() } );
209+
return b;
210+
}
211+
const callback = dummyScopeAndNS( () => withScope( gen ) );
212+
const result = await callback();
213+
214+
expect( result ).toBe( 15 );
215+
steps.forEach( ( step ) => {
216+
expect( step.scope ).toBe( dummyScope );
217+
expect( step.namespace ).toBe( dummyNamespace );
218+
} );
219+
expect( getScope() ).toBe( previousScope );
220+
expect( getNamespace() ).toBe( previousNamespace );
221+
} );
222+
223+
it( 'should handle rejected promises within a generator function and throw after yielding', async () => {
224+
function* gen() {
225+
const a = yield Promise.resolve( 2 );
226+
yield Promise.reject( new Error( 'FinalReject' ) );
227+
return a;
228+
}
229+
const wrapped = dummyScopeAndNS( () => withScope( gen ) );
230+
await expect( wrapped() ).rejects.toThrow( 'FinalReject' );
231+
expect( getScope() ).toBe( previousScope );
232+
expect( getNamespace() ).toBe( previousNamespace );
233+
} );
234+
} );
28235
} );

‎packages/interactivity/src/utils.ts

+11-9
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,15 @@ export function withScope( func: ( ...args: unknown[] ) => unknown ) {
150150
const gen = func( ...args ) as Generator;
151151
let value: any;
152152
let it: any;
153+
let error: any;
153154
while ( true ) {
154155
setNamespace( ns );
155156
setScope( scope );
156157
try {
157-
it = gen.next( value );
158+
it = error ? gen.throw( error ) : gen.next( value );
159+
error = undefined;
160+
} catch ( e ) {
161+
throw e;
158162
} finally {
159163
resetScope();
160164
resetNamespace();
@@ -163,16 +167,14 @@ export function withScope( func: ( ...args: unknown[] ) => unknown ) {
163167
try {
164168
value = await it.value;
165169
} catch ( e ) {
166-
setNamespace( ns );
167-
setScope( scope );
168-
gen.throw( e );
169-
} finally {
170-
resetScope();
171-
resetNamespace();
170+
error = e;
172171
}
173-
174172
if ( it.done ) {
175-
break;
173+
if ( error ) {
174+
throw error;
175+
} else {
176+
break;
177+
}
176178
}
177179
}
178180

‎test/e2e/specs/interactivity/async-actions.spec.ts

+30
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,34 @@ test.describe( 'async actions', () => {
2828
await page.getByTestId( 'reject' ).click();
2929
await expect( resultInput ).toHaveValue( 'Error: 😘' );
3030
} );
31+
32+
test( 'Promise generator callbacks should yield the correct value after captured errors', async ( {
33+
page,
34+
} ) => {
35+
const resultInput = page.getByTestId( 'result' );
36+
await expect( resultInput ).toHaveValue( '' );
37+
38+
await page.getByTestId( 'capture' ).click();
39+
await expect( resultInput ).toHaveValue( '3' );
40+
} );
41+
42+
test( 'Promise generator callbacks should be able to throw errors', async ( {
43+
page,
44+
} ) => {
45+
const resultInput = page.getByTestId( 'result' );
46+
await expect( resultInput ).toHaveValue( '' );
47+
48+
await page.getByTestId( 'captureThrow' ).click();
49+
await expect( resultInput ).toHaveValue( 'Error: 🤯' );
50+
} );
51+
52+
test( 'Promise generator callbacks should throw when rejected promises are returned', async ( {
53+
page,
54+
} ) => {
55+
const resultInput = page.getByTestId( 'result' );
56+
await expect( resultInput ).toHaveValue( '' );
57+
58+
await page.getByTestId( 'captureReturnReject' ).click();
59+
await expect( resultInput ).toHaveValue( 'Error: 🔚' );
60+
} );
3161
} );

0 commit comments

Comments
 (0)