Skip to content

Commit 9d2d759

Browse files
committed
fix: prevent user callback to be called twice when it throws an error
1 parent 3b98d89 commit 9d2d759

File tree

2 files changed

+73
-4
lines changed

2 files changed

+73
-4
lines changed

packages/cbjs/src/utilities.ts

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ export { type Cas, CouchbaseCas };
3636
export type NodeCallback<T> = (...args: [null, T] | [Error, null]) => void;
3737
export type VoidNodeCallback = (err: Error | null) => void;
3838

39+
class UserCallbackError extends Error {}
40+
3941
/**
4042
* @internal
4143
*/
@@ -61,7 +63,21 @@ export class PromiseHelper {
6163
const prom = logicFn();
6264

6365
if (callback) {
64-
prom.then((res) => callback(null, res)).catch((err) => callback(err, null));
66+
prom
67+
.then((res) => {
68+
try {
69+
callback(null, res);
70+
} catch (err) {
71+
throw new UserCallbackError('', { cause: err });
72+
}
73+
})
74+
.catch((err) => {
75+
if (err instanceof UserCallbackError) {
76+
throw err.cause;
77+
}
78+
79+
callback(err, null);
80+
});
6581
}
6682

6783
return prom;
@@ -70,10 +86,10 @@ export class PromiseHelper {
7086
/**
7187
* @internal
7288
*/
73-
static wrap<T>(
89+
static wrap(
7490
logicFn: (callback: VoidNodeCallback) => void,
7591
callback?: VoidNodeCallback | null
76-
): Promise<T>;
92+
): Promise<void>;
7793
static wrap<T>(
7894
logicFn: (callback: NodeCallback<NonVoid<T>>) => void,
7995
callback?: NodeCallback<NonVoid<T>> | null
@@ -94,7 +110,21 @@ export class PromiseHelper {
94110
});
95111

96112
if (callback) {
97-
prom.then((res) => callback(null, res)).catch((err) => callback(err, null));
113+
prom
114+
.then((res) => {
115+
try {
116+
callback(null, res);
117+
} catch (err) {
118+
throw new UserCallbackError('', { cause: err });
119+
}
120+
})
121+
.catch((err) => {
122+
if (err instanceof UserCallbackError) {
123+
throw err.cause;
124+
}
125+
126+
callback(err, null);
127+
});
98128
}
99129

100130
return prom;

tests/cbjs/tests/utilities.spec.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,16 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17+
import { setImmediate } from 'node:timers/promises';
1718
import { describe, vi } from 'vitest';
1819

1920
import { CouchbaseError } from '@cbjsdev/cbjs';
21+
import { PromiseHelper } from '@cbjsdev/cbjs/internal';
2022
import { invariant, waitFor } from '@cbjsdev/shared';
2123
import { createCouchbaseTest } from '@cbjsdev/vitest';
2224

25+
import { NodeCallback } from './kv.durability.spec.js';
26+
2327
describe('PromiseHelper', async () => {
2428
const test = await createCouchbaseTest();
2529

@@ -62,6 +66,23 @@ describe('PromiseHelper', async () => {
6266
expect(callback).toHaveBeenCalled();
6367
});
6468
});
69+
70+
// This test is expected to produce an unhandled error
71+
test('user callback should not be called twice when it throws an error', async ({
72+
expect,
73+
}) => {
74+
const userCallback = vi.fn((err, res) => {
75+
throw new Error('user callback error');
76+
});
77+
78+
await PromiseHelper.wrap((cb) => {
79+
cb(null, 'success');
80+
}, userCallback);
81+
82+
await setImmediate();
83+
84+
expect(userCallback).toHaveBeenCalledTimes(1);
85+
});
6586
});
6687

6788
describe.skip('wrapAsync', async () => {
@@ -105,3 +126,21 @@ describe('PromiseHelper', async () => {
105126
});
106127
});
107128
});
129+
130+
/**
131+
* prom
132+
* .then((res) => {
133+
* try {
134+
* callback(null, res);
135+
* } catch (err) {
136+
* throw new UserCallbackError('', { cause: err });
137+
* }
138+
* })
139+
* .catch((err) => {
140+
* if (err instanceof UserCallbackError) {
141+
* throw err.cause;
142+
* }
143+
*
144+
* callback(err, null);
145+
* });
146+
*/

0 commit comments

Comments
 (0)