Skip to content

Commit 4185b4a

Browse files
authored
fix(client): isolate per-userkey failures in resolveUserKeysInHtml (#129)
Switch from Promise.all to Promise.allSettled when fetching user info for ri:user references. A single rejected getUserByKey call previously aborted resolution for the entire page; now each failure falls back to the raw userkey (matching getUserByKey's own caught-error fallback) without affecting the other resolutions. Add a regression test that spies on getUserByKey to force one rejection alongside one success and verifies partial replacement still happens.
1 parent cde5798 commit 4185b4a

2 files changed

Lines changed: 35 additions & 6 deletions

File tree

lib/confluence-client.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -496,14 +496,23 @@ class ConfluenceClient {
496496
return { html, userMap: new Map() };
497497
}
498498

499-
// Fetch user info for all keys in parallel
500-
const userPromises = Array.from(userKeys).map(key => this.getUserByKey(key));
501-
const users = await Promise.all(userPromises);
499+
// Fetch user info for all keys in parallel; isolate per-key failures so a
500+
// single rejection doesn't drop display names for the rest of the page.
501+
const keysArray = Array.from(userKeys);
502+
const results = await Promise.allSettled(
503+
keysArray.map(key => this.getUserByKey(key))
504+
);
502505

503-
// Build userkey -> displayName map
506+
// Build userkey -> displayName map. On rejection, fall back to the raw
507+
// userkey (mirrors getUserByKey's own caught-error fallback).
504508
const userMap = new Map();
505-
users.forEach(user => {
506-
userMap.set(user.key, user.displayName);
509+
results.forEach((result, i) => {
510+
const key = keysArray[i];
511+
if (result.status === 'fulfilled') {
512+
userMap.set(result.value.key, result.value.displayName);
513+
} else {
514+
userMap.set(key, key);
515+
}
507516
});
508517

509518
// Replace userkey references with display names in HTML

tests/confluence-client.test.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,6 +1268,26 @@ describe('ConfluenceClient', () => {
12681268
expect(resolved).toBe(html);
12691269
expect(userMap.size).toBe(0);
12701270
});
1271+
1272+
test('should isolate per-key failures: one rejection does not drop other resolutions', async () => {
1273+
const spy = jest.spyOn(client, 'getUserByKey').mockImplementation(async (key) => {
1274+
if (key === 'good') {
1275+
return { key, displayName: 'Good User', username: 'good' };
1276+
}
1277+
throw new Error('synthetic failure');
1278+
});
1279+
1280+
const html =
1281+
'<p><ac:link><ri:user ri:userkey="good" /></ac:link> and ' +
1282+
'<ac:link><ri:user ri:userkey="bad" /></ac:link></p>';
1283+
const { html: resolved, userMap } = await client.resolveUserKeysInHtml(html);
1284+
1285+
expect(resolved).toBe('<p>@Good User and @bad</p>');
1286+
expect(userMap.get('good')).toBe('Good User');
1287+
expect(userMap.get('bad')).toBe('bad');
1288+
1289+
spy.mockRestore();
1290+
});
12711291
});
12721292

12731293
describe('page creation and updates', () => {

0 commit comments

Comments
 (0)