paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 36 Packages Projects Releases Wiki Activity

Merge pull request #33187 from lokeshkaki/fix/scheduler-abort-signal-msgraph

Browse Source 
fix(catalog): propagate AbortSignal in MicrosoftGraphOrgEntityProvider to fix stuck scheduler task
This commit is contained in:
Fredrik Adelöw
2026-03-09 12:09:24 +01:00
committed by GitHub
parent 83af2e3ffa df865b64a8
commit dd59aeb842
7 changed files with 237 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/fluffy-colts-stop.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend-module-msgraph': patch
---
Fixed scheduler task remaining stuck in running state after pod termination by propagating AbortSignal into MicrosoftGraphOrgEntityProvider.read()
plugins/catalog-backend-module-msgraph/report.api.md
+16 -2
View File
@@ -78,6 +78,7 @@ export class MicrosoftGraphClient {
groupId: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
signal?: AbortSignal,
): AsyncIterable<GroupMember>;
// (undocumented)
getGroupPhoto(groupId: string, sizeId?: string): Promise<string | undefined>;
@@ -89,13 +90,18 @@ export class MicrosoftGraphClient {
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
path?: string,
signal?: AbortSignal,
): AsyncIterable<MicrosoftGraph.Group>;
getGroupUserMembers(
groupId: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
signal?: AbortSignal,
): AsyncIterable<MicrosoftGraph.User>;
getOrganization(tenantId: string): Promise<MicrosoftGraph.Organization>;
getOrganization(
tenantId: string,
signal?: AbortSignal,
): Promise<MicrosoftGraph.Organization>;
// (undocumented)
getUserPhoto(userId: string, sizeId?: string): Promise<string | undefined>;
getUserPhotoWithSizeLimit(
@@ -106,21 +112,25 @@ export class MicrosoftGraphClient {
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
path?: string,
signal?: AbortSignal,
): AsyncIterable<MicrosoftGraph.User>;
requestApi(
path: string,
query?: ODataQuery,
headers?: Record<string, string>,
signal?: AbortSignal,
): Promise<Response>;
requestCollection<T>(
path: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
signal?: AbortSignal,
): AsyncIterable<T>;
requestRaw(
url: string,
headers?: Record<string, string>,
retryCount?: number,
signal?: AbortSignal,
): Promise<Response>;
}
@@ -142,7 +152,10 @@ export class MicrosoftGraphOrgEntityProvider implements EntityProvider {
options: MicrosoftGraphOrgEntityProviderOptions,
): MicrosoftGraphOrgEntityProvider[];
getProviderName(): string;
read(options?: { logger?: LoggerService }): Promise<void>;
read(options?: {
logger?: LoggerService;
signal?: AbortSignal;
}): Promise<void>;
}
// @public @deprecated
@@ -304,6 +317,7 @@ export function readMicrosoftGraphOrg(
groupTransformer?: GroupTransformer;
organizationTransformer?: OrganizationTransformer;
logger: LoggerService;
signal?: AbortSignal;
},
): Promise<{
users: UserEntity[];
plugins/catalog-backend-module-msgraph/src/microsoftGraph/client.ts
+39 -6
View File
@@ -128,6 +128,7 @@ export class MicrosoftGraphClient {
path: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
signal?: AbortSignal,
): AsyncIterable<T> {
// upgrade to advanced query mode transparently when "search" is used
// to stay backwards compatible.
@@ -151,7 +152,7 @@ export class MicrosoftGraphClient {
}
: {};
let response = await this.requestApi(path, query, headers);
let response = await this.requestApi(path, query, headers, signal);
for (;;) {
if (response.status !== 200) {
@@ -170,7 +171,12 @@ export class MicrosoftGraphClient {
return;
}
response = await this.requestRaw(result['@odata.nextLink'], headers);
response = await this.requestRaw(
result['@odata.nextLink'],
headers,
2,
signal,
);
}
}
@@ -186,6 +192,7 @@ export class MicrosoftGraphClient {
path: string,
query?: ODataQuery,
headers?: Record<string, string>,
signal?: AbortSignal,
): Promise<Response> {
const queryString = qs.stringify(
{
@@ -205,6 +212,8 @@ export class MicrosoftGraphClient {
return await this.requestRaw(
`${this.baseUrl}/${path}${queryString}`,
headers,
2,
signal,
);
}
@@ -218,6 +227,7 @@ export class MicrosoftGraphClient {
url: string,
headers?: Record<string, string>,
retryCount = 2,
signal?: AbortSignal,
): Promise<Response> {
// Make sure that we always have a valid access token (might be cached)
const urlObj = new URL(url);
@@ -235,10 +245,11 @@ export class MicrosoftGraphClient {
...headers,
Authorization: `Bearer ${token.token}`,
},
signal,
});
} catch (e: any) {
if (e?.code === 'ETIMEDOUT' && retryCount > 0) {
return this.requestRaw(url, headers, retryCount - 1);
return this.requestRaw(url, headers, retryCount - 1, signal);
}
throw e;
}
@@ -280,8 +291,14 @@ export class MicrosoftGraphClient {
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
path: string = 'users',
signal?: AbortSignal,
): AsyncIterable<MicrosoftGraph.User> {
yield* this.requestCollection<MicrosoftGraph.User>(path, query, queryMode);
yield* this.requestCollection<MicrosoftGraph.User>(
path,
query,
queryMode,
signal,
);
}
/**
@@ -320,8 +337,14 @@ export class MicrosoftGraphClient {
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
path: string = 'groups',
signal?: AbortSignal,
): AsyncIterable<MicrosoftGraph.Group> {
yield* this.requestCollection<MicrosoftGraph.Group>(path, query, queryMode);
yield* this.requestCollection<MicrosoftGraph.Group>(
path,
query,
queryMode,
signal,
);
}
/**
@@ -336,11 +359,13 @@ export class MicrosoftGraphClient {
groupId: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
signal?: AbortSignal,
): AsyncIterable<GroupMember> {
yield* this.requestCollection<GroupMember>(
`groups/${groupId}/members`,
query,
queryMode,
signal,
);
}
@@ -357,11 +382,13 @@ export class MicrosoftGraphClient {
groupId: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
signal?: AbortSignal,
): AsyncIterable<MicrosoftGraph.User> {
yield* this.requestCollection<MicrosoftGraph.User>(
`groups/${groupId}/members/microsoft.graph.user/`,
query,
queryMode,
signal,
);
}
@@ -374,8 +401,14 @@ export class MicrosoftGraphClient {
*/
async getOrganization(
tenantId: string,
signal?: AbortSignal,
): Promise<MicrosoftGraph.Organization> {
const response = await this.requestApi(`organization/${tenantId}`);
const response = await this.requestApi(
`organization/${tenantId}`,
undefined,
undefined,
signal,
);
if (response.status !== 200) {
await this.handleError(`organization/${tenantId}`, response);
plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts
+92 -23
View File
@@ -140,6 +140,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledWith(
@@ -188,6 +189,7 @@ describe('read microsoft graph', () => {
},
'advanced',
undefined,
undefined,
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledWith(
@@ -232,6 +234,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledWith(
@@ -280,6 +283,7 @@ describe('read microsoft graph', () => {
},
undefined,
'/users/x/y',
undefined,
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledWith(
@@ -332,6 +336,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroupUserMembers).toHaveBeenCalledTimes(1);
@@ -339,6 +344,7 @@ describe('read microsoft graph', () => {
'groupid',
{ top: 999 },
undefined,
undefined,
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
@@ -391,6 +397,7 @@ describe('read microsoft graph', () => {
},
'advanced',
undefined,
undefined,
);
expect(client.getGroupUserMembers).toHaveBeenCalledTimes(1);
@@ -398,6 +405,7 @@ describe('read microsoft graph', () => {
'groupid',
{ top: 999 },
'advanced',
undefined,
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
@@ -447,6 +455,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroupUserMembers).toHaveBeenCalledTimes(1);
@@ -457,6 +466,7 @@ describe('read microsoft graph', () => {
top: 999,
},
undefined,
undefined,
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
@@ -509,6 +519,7 @@ describe('read microsoft graph', () => {
},
undefined,
'/groups/x/y',
undefined,
);
});
});
@@ -545,7 +556,10 @@ describe('read microsoft graph', () => {
);
expect(client.getOrganization).toHaveBeenCalledTimes(1);
expect(client.getOrganization).toHaveBeenCalledWith('tenantid');
expect(client.getOrganization).toHaveBeenCalledWith(
'tenantid',
undefined,
);
});
it('should read organization with custom transformer', async () => {
@@ -563,7 +577,10 @@ describe('read microsoft graph', () => {
expect(rootGroup).toEqual(undefined);
expect(client.getOrganization).toHaveBeenCalledTimes(1);
expect(client.getOrganization).toHaveBeenCalledWith('tenantid');
expect(client.getOrganization).toHaveBeenCalledWith(
'tenantid',
undefined,
);
});
});
@@ -636,11 +653,17 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
expect(client.getGroupMembers).toHaveBeenCalledWith(
'groupid',
{
top: 999,
},
undefined,
undefined,
);
// TODO: Loading groups photos doesn't work right now as Microsoft Graph
// doesn't allows this yet
// expect(client.getGroupPhotoWithSizeLimit).toBeCalledTimes(1);
@@ -716,11 +739,17 @@ describe('read microsoft graph', () => {
},
'advanced',
undefined,
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
expect(client.getGroupMembers).toHaveBeenCalledWith(
'groupid',
{
top: 999,
},
undefined,
undefined,
);
// TODO: Loading groups photos doesn't work right now as Microsoft Graph
// doesn't allows this yet
// expect(client.getGroupPhotoWithSizeLimit).toBeCalledTimes(1);
@@ -797,11 +826,17 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
expect(client.getGroupMembers).toHaveBeenCalledWith(
'groupid',
{
top: 999,
},
undefined,
undefined,
);
// TODO: Loading groups photos doesn't work right now as Microsoft Graph
// doesn't allows this yet
// expect(client.getGroupPhotoWithSizeLimit).toBeCalledTimes(1);
@@ -871,11 +906,17 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
expect(client.getGroupMembers).toHaveBeenCalledWith(
'groupid',
{
top: 999,
},
undefined,
undefined,
);
});
it('should read groups and their sub groups', async () => {
@@ -976,14 +1017,25 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(2);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
expect(client.getGroupMembers).toHaveBeenCalledWith('childgroupid', {
top: 999,
});
expect(client.getGroupMembers).toHaveBeenCalledWith(
'groupid',
{
top: 999,
},
undefined,
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledWith(
'childgroupid',
{
top: 999,
},
undefined,
undefined,
);
// TODO: Loading groups photos doesn't work right now as Microsoft Graph
// doesn't allows this yet
// expect(client.getGroupPhotoWithSizeLimit).toBeCalledTimes(1);
@@ -1056,11 +1108,17 @@ describe('read microsoft graph', () => {
},
undefined,
'/groups/x/y',
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
expect(client.getGroupMembers).toHaveBeenCalledWith(
'groupid',
{
top: 999,
},
undefined,
undefined,
);
});
});
@@ -1211,6 +1269,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroups).toHaveBeenCalledTimes(1);
expect(client.getGroups).toHaveBeenCalledWith(
@@ -1220,6 +1279,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
});
@@ -1253,6 +1313,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroups).toHaveBeenCalledTimes(1);
expect(client.getGroups).toHaveBeenCalledWith(
@@ -1262,6 +1323,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
});
@@ -1293,6 +1355,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroups).toHaveBeenCalledTimes(1);
expect(client.getGroups).toHaveBeenCalledWith(
@@ -1301,6 +1364,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
});
@@ -1331,6 +1395,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
});
@@ -1369,6 +1434,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroups).toHaveBeenNthCalledWith(
2,
@@ -1378,6 +1444,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
});
@@ -1419,6 +1486,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
expect(client.getGroups).toHaveBeenCalledTimes(1);
expect(client.getGroups).toHaveBeenCalledWith(
@@ -1427,6 +1495,7 @@ describe('read microsoft graph', () => {
},
undefined,
undefined,
undefined,
);
});
});
plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts
+24 -5
View File
@@ -53,6 +53,7 @@ export async function readMicrosoftGraphUsers(
loadUserPhotos?: boolean;
transformer?: UserTransformer;
logger: LoggerService;
signal?: AbortSignal;
},
): Promise<{
users: UserEntity[]; // With all relations empty
@@ -66,6 +67,7 @@ export async function readMicrosoftGraphUsers(
},
options.queryMode,
options.userPath,
options.signal,
);
return {
@@ -93,6 +95,7 @@ export async function readMicrosoftGraphUsersInGroups(
groupExpand?: string;
transformer?: UserTransformer;
logger: LoggerService;
signal?: AbortSignal;
},
): Promise<{
users: UserEntity[]; // With all relations empty
@@ -112,6 +115,7 @@ export async function readMicrosoftGraphUsersInGroups(
},
options.queryMode,
options.userGroupMemberPath,
options.signal,
)) {
// Process all groups in parallel, otherwise it can take quite some time
userGroupMemberPromises.push(
@@ -126,6 +130,7 @@ export async function readMicrosoftGraphUsersInGroups(
top: PAGE_SIZE,
},
options.queryMode,
options.signal,
)) {
userGroupMembers.set(user.id!, user);
groupMemberCount++;
@@ -161,12 +166,12 @@ export async function readMicrosoftGraphUsersInGroups(
export async function readMicrosoftGraphOrganization(
client: MicrosoftGraphClient,
tenantId: string,
options?: { transformer?: OrganizationTransformer },
options?: { transformer?: OrganizationTransformer; signal?: AbortSignal },
): Promise<{
rootGroup?: GroupEntity; // With all relations empty
}> {
// For now we expect a single root organization
const organization = await client.getOrganization(tenantId);
const organization = await client.getOrganization(tenantId, options?.signal);
const transformer = options?.transformer ?? defaultOrganizationTransformer;
const rootGroup = await transformer(organization);
@@ -186,6 +191,7 @@ export async function readMicrosoftGraphGroups(
groupIncludeSubGroups?: boolean;
groupTransformer?: GroupTransformer;
organizationTransformer?: OrganizationTransformer;
signal?: AbortSignal;
},
): Promise<{
groups: GroupEntity[]; // With all relations empty
@@ -200,6 +206,7 @@ export async function readMicrosoftGraphGroups(
const { rootGroup } = await readMicrosoftGraphOrganization(client, tenantId, {
transformer: options?.organizationTransformer,
signal: options?.signal,
});
if (rootGroup) {
groupMember.set(rootGroup.metadata.name, new Set<string>());
@@ -219,6 +226,7 @@ export async function readMicrosoftGraphGroups(
},
options?.queryMode,
options?.groupPath,
options?.signal,
)) {
// Process all groups in parallel, otherwise it can take quite some time
promises.push(
@@ -238,9 +246,14 @@ export async function readMicrosoftGraphGroups(
return;
}
for await (const member of client.getGroupMembers(group.id!, {
top: PAGE_SIZE,
})) {
for await (const member of client.getGroupMembers(
group.id!,
{
top: PAGE_SIZE,
},
undefined,
options?.signal,
)) {
if (!member.id) {
continue;
}
@@ -261,6 +274,8 @@ export async function readMicrosoftGraphGroups(
for await (const subMember of client.getGroupMembers(
member.id!,
{ top: PAGE_SIZE },
undefined,
options?.signal,
)) {
if (!subMember.id) {
continue;
@@ -425,6 +440,7 @@ export async function readMicrosoftGraphOrg(
groupTransformer?: GroupTransformer;
organizationTransformer?: OrganizationTransformer;
logger: LoggerService;
signal?: AbortSignal;
},
): Promise<{ users: UserEntity[]; groups: GroupEntity[] }> {
let users: UserEntity[] = [];
@@ -447,6 +463,7 @@ export async function readMicrosoftGraphOrg(
loadUserPhotos: options.loadUserPhotos,
transformer: options.userTransformer,
logger: options.logger,
signal: options.signal,
},
);
users = usersInGroups;
@@ -460,6 +477,7 @@ export async function readMicrosoftGraphOrg(
loadUserPhotos: options.loadUserPhotos,
transformer: options.userTransformer,
logger: options.logger,
signal: options.signal,
});
users = usersWithFilter;
}
@@ -474,6 +492,7 @@ export async function readMicrosoftGraphOrg(
groupIncludeSubGroups: options.groupIncludeSubGroups,
groupTransformer: options.groupTransformer,
organizationTransformer: options.organizationTransformer,
signal: options.signal,
});
resolveRelations(rootGroup, groups, users, groupMember, groupMemberOf);
plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.test.ts
+51
View File
@@ -255,6 +255,57 @@ describe('MicrosoftGraphOrgEntityProvider', () => {
);
});
it('should stop processing when AbortSignal is aborted', async () => {
jest.spyOn(logger, 'child').mockReturnValue(logger as any);
const config = new ConfigReader({
catalog: {
providers: {
microsoftGraphOrg: {
customProviderId: {
target: 'target',
tenantId: 'tenantId',
clientId: 'clientId',
clientSecret: 'clientSecret',
},
},
},
},
});
const localTaskRunner = new PersistingTaskRunner();
const provider = MicrosoftGraphOrgEntityProvider.fromConfig(config, {
logger,
schedule: localTaskRunner,
})[0];
await provider.connect(entityProviderConnection);
const controller = new AbortController();
controller.abort();
readMicrosoftGraphOrgMocked.mockImplementationOnce(
async (_client, _tenantId, options) => {
if (options.signal?.aborted) {
const error = new Error('The operation was aborted');
error.name = 'AbortError';
throw error;
}
return { users: [], groups: [] };
},
);
const taskDef = localTaskRunner.getTasks()[0];
// Should resolve without throwing (abort is caught and logged at debug level)
await (taskDef.fn as (signal: AbortSignal) => Promise<void>)(
controller.signal,
);
expect(entityProviderConnection.applyMutation).not.toHaveBeenCalled();
expect(logger.debug).toHaveBeenCalledWith(
expect.stringContaining('refresh aborted due to shutdown'),
);
});
it('fail without schedule and scheduler', () => {
const config = new ConfigReader({
catalog: {
plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts
+10 -3
View File
@@ -326,7 +326,7 @@ export class MicrosoftGraphOrgEntityProvider implements EntityProvider {
* Runs one complete ingestion loop. Call this method regularly at some
* appropriate cadence.
*/
async read(options?: { logger?: LoggerService }) {
async read(options?: { logger?: LoggerService; signal?: AbortSignal }) {
if (!this.connection) {
throw new Error('Not initialized');
}
@@ -361,6 +361,7 @@ export class MicrosoftGraphOrgEntityProvider implements EntityProvider {
userTransformer: this.options.userTransformer,
organizationTransformer: this.options.organizationTransformer,
logger: logger,
signal: options?.signal,
},
);
@@ -386,7 +387,7 @@ export class MicrosoftGraphOrgEntityProvider implements EntityProvider {
const id = `${this.getProviderName()}:refresh`;
await taskRunner.run({
id,
fn: async () => {
fn: async (abortSignal: AbortSignal) => {
const logger = this.options.logger.child({
class: MicrosoftGraphOrgEntityProvider.prototype.constructor.name,
taskId: id,
@@ -394,8 +395,14 @@ export class MicrosoftGraphOrgEntityProvider implements EntityProvider {
});
try {
await this.read({ logger });
await this.read({ logger, signal: abortSignal });
} catch (error) {
if (error instanceof Error && error.name === 'AbortError') {
logger.debug(
`${this.getProviderName()} refresh aborted due to shutdown`,
);
return;
}
logger.error(
`${this.getProviderName()} refresh failed, ${error}`,
error,