rayan/bruno
1
0
Fork 0
mirror of https://github.com/usebruno/bruno.git synced 2026-06-11 09:51:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: cookie wrapper callback mode returns never-resolving Promise (#7442)

Browse Source 
* fix: cookie wrapper callback mode returns never-resolving Promise

tough-cookie's createPromiseCallback() intentionally never resolves the
returned Promise when a callback is provided — only the callback fires.
The cookie jar wrapper was propagating this never-resolving Promise via
`return cookieJar.getCookies(url, cb)` in callback-mode paths. When user
scripts did `await jar.getCookie(url, name, callback)` in the Node VM
(developer sandbox), the await hung forever, blocking the CLI runner.

Fix: drop the return value in all callback-mode paths so the wrapper
returns void (undefined). `await undefined` resolves immediately.

Affected methods: getCookie, getCookies, hasCookie, clear, deleteCookies.

* fix: validation-error callback paths also return void instead of callback result

The validation guards (e.g. missing URL) did `return callback(error)` which
leaks whatever the user's callback returns. Apply the same pattern used for
the main callback paths: call the callback, then return void.

Also makes deleteCookie's `return executeDelete(callback)` consistent
(executeDelete already returns void, but the explicit pattern is clearer).
This commit is contained in:
lohit
2026-03-11 15:26:09 +00:00
committed by GitHub
parent e7c2c7c872
commit 7c58740c74
2 changed files with 93 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
packages/bruno-requests/src/cookies/index.spec.ts
View File

@@ -155,6 +155,69 @@ describe('Bruno Cookie Jar Wrapper - API Examples', () => {
});
});
describe('Callback mode does not return a Promise', () => {
// tough-cookie's createPromiseCallback() returns a never-resolving Promise when
// a callback is provided. The wrapper must NOT propagate that Promise, otherwise
// `await jar.getCookie(url, name, cb)` hangs forever (blocks the Node VM runner).
test('getCookie with callback returns void, not a Promise', () => {
jar.setCookie(testUrl, 'x', '1');
const ret = jar.getCookie(testUrl, 'x', () => {});
expect(ret).toBeUndefined();
});
test('getCookies with callback returns void, not a Promise', () => {
const ret = jar.getCookies(testUrl, () => {});
expect(ret).toBeUndefined();
});
test('hasCookie with callback returns void, not a Promise', () => {
const ret = jar.hasCookie(testUrl, 'x', () => {});
expect(ret).toBeUndefined();
});
test('clear with callback returns void, not a Promise', () => {
const ret = jar.clear(() => {});
expect(ret).toBeUndefined();
});
test('deleteCookies with callback returns void, not a Promise', () => {
const ret = jar.deleteCookies(testUrl, () => {});
expect(ret).toBeUndefined();
});
test('deleteCookie with callback returns void, not a Promise', () => {
const ret = jar.deleteCookie(testUrl, 'x', () => {});
expect(ret).toBeUndefined();
});
test('validation-error paths with callback return void, not the callback return value', () => {
// If the wrapper did `return callback(error)`, the caller would receive
// whatever the callback returns — which could be a truthy value or a Promise.
// All validation-error callback paths must return void (undefined).
const spy = () => 'leaked!' as any;
expect(jar.getCookie('', '', spy)).toBeUndefined();
expect(jar.getCookies('', spy)).toBeUndefined();
expect(jar.hasCookie('', '', spy)).toBeUndefined();
expect(jar.deleteCookies('', spy)).toBeUndefined();
expect(jar.deleteCookie('', '', spy)).toBeUndefined();
});
test('getCookie with callback can be safely awaited without hanging', async () => {
await jar.setCookie(testUrl, 'token', 'abc');
let callbackData: any = null;
// This would hang forever before the fix if getCookie returned tough-cookie's Promise
await jar.getCookie(testUrl, 'token', (_err, cookie) => {
callbackData = cookie;
});
expect(callbackData).not.toBeNull();
expect(callbackData.key).toBe('token');
expect(callbackData.value).toBe('abc');
});
});
describe('Error Handling', () => {
test('setCookie handles missing URL', async () => {
await expect(jar.setCookie('', 'name', 'value')).rejects.toThrow('URL is required');

42
packages/bruno-requests/src/cookies/index.ts
View File

@@ -189,18 +189,23 @@ const cookieJarWrapper = () => {
) {
if (!url || !cookieName) {
const error = new Error('URL and cookie name are required');
if (callback) return callback(error);
if (callback) {
callback(error); return;
}
return Promise.reject(error);
}
if (callback) {
// Callback mode
return cookieJar.getCookies(url, (err: Error | null, cookies?: Cookie[]) => {
// Callback mode – do NOT return the value from cookieJar.getCookies() because
// tough-cookie returns a never-resolving Promise when a callback is provided.
// Returning void ensures `await` on a callback-style call resolves immediately.
cookieJar.getCookies(url, (err: Error | null, cookies?: Cookie[]) => {
if (err) return callback(err);
const cookieList = cookies || [];
const cookie = cookieList.find((c) => c.key === cookieName);
callback(null, cookie || null);
});
return;
}
// Promise mode
@@ -222,16 +227,19 @@ const cookieJarWrapper = () => {
) {
if (!url || !cookieName) {
const error = new Error('URL and cookie name are required');
if (callback) return callback(error);
if (callback) {
callback(error); return;
}
return Promise.reject(error);
}
if (callback) {
return cookieJar.getCookies(url, (err: Error | null, cookies?: Cookie[]) => {
cookieJar.getCookies(url, (err: Error | null, cookies?: Cookie[]) => {
if (err) return callback(err);
const cookieList = cookies || [];
callback(null, cookieList.some((c) => c.key === cookieName));
});
return;
}
return new Promise<boolean>((resolve, reject) => {
@@ -247,13 +255,16 @@ const cookieJarWrapper = () => {
getCookies: function (url: string, callback?: (err: Error | null | undefined, cookies?: Cookie[]) => void) {
if (!url) {
const error = new Error('URL is required');
if (callback) return callback(error);
if (callback) {
callback(error); return;
}
return Promise.reject(error);
}
if (callback) {
// Callback mode
return cookieJar.getCookies(url, callback as any);
cookieJar.getCookies(url, callback as any);
return;
}
// Promise mode
@@ -396,7 +407,8 @@ const cookieJarWrapper = () => {
clear: function (callback?: (err?: Error | undefined) => void) {
if (callback) {
// Callback mode
return (cookieJar as any).store.removeAllCookies(callback);
(cookieJar as any).store.removeAllCookies(callback);
return;
}
// Promise mode
@@ -411,13 +423,15 @@ const cookieJarWrapper = () => {
deleteCookies: function (url: string, callback?: (err?: Error | undefined) => void) {
if (!url) {
const error = new Error('URL is required');
if (callback) return callback(error);
if (callback) {
callback(error); return;
}
return Promise.reject(error);
}
if (callback) {
// Callback mode
return cookieJar.getCookies(url, (err: Error | null, cookies?: Cookie[]) => {
cookieJar.getCookies(url, (err: Error | null, cookies?: Cookie[]) => {
if (err) return callback(err);
const cookieList = cookies || [];
if (!cookieList.length) return callback(undefined);
@@ -434,6 +448,7 @@ const cookieJarWrapper = () => {
(cookieJar as any).store.removeCookie(cookie.domain, cookie.path, cookie.key, done);
});
});
return;
}
// Promise mode
@@ -461,7 +476,9 @@ const cookieJarWrapper = () => {
deleteCookie: function (url: string, cookieName: string, callback?: (err?: Error | undefined) => void) {
if (!url || !cookieName) {
const error = new Error('URL and cookie name are required');
if (callback) return callback(error);
if (callback) {
callback(error); return;
}
return Promise.reject(error);
}
@@ -496,7 +513,8 @@ const cookieJarWrapper = () => {
if (callback) {
// Callback mode
return executeDelete(callback);
executeDelete(callback);
return;
}
// Promise mode