refactor(core): Enforce range for shutdown priority (no-changelog) (#9944)

This commit is contained in:
Iván Ovejero 2024-07-04 20:26:11 +02:00 committed by GitHub
parent c82579bf76
commit 757feaf585
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 42 additions and 9 deletions

View file

@ -9,7 +9,8 @@ import {
} from 'n8n-workflow';
import { ActiveExecutions } from '@/ActiveExecutions';
import config from '@/config';
import { HIGHEST_PRIORITY, OnShutdown } from './decorators/OnShutdown';
import { OnShutdown } from './decorators/OnShutdown';
import { HIGHEST_SHUTDOWN_PRIORITY } from './constants';
export type JobId = Bull.JobId;
export type Job = Bull.Job<JobData>;
@ -109,7 +110,7 @@ export class Queue {
return await this.jobQueue.client.ping();
}
@OnShutdown(HIGHEST_PRIORITY)
@OnShutdown(HIGHEST_SHUTDOWN_PRIORITY)
// Stop accepting new jobs, `doNotWaitActive` allows reporting progress
async pause(): Promise<void> {
return await this.jobQueue?.pause(true, true);

View file

@ -165,3 +165,7 @@ export const ARTIFICIAL_TASK_DATA = {
],
],
};
export const LOWEST_SHUTDOWN_PRIORITY = 0;
export const DEFAULT_SHUTDOWN_PRIORITY = 100;
export const HIGHEST_SHUTDOWN_PRIORITY = 200;

View file

@ -1,10 +1,7 @@
import { Container } from 'typedi';
import { ApplicationError } from 'n8n-workflow';
import { type ServiceClass, ShutdownService } from '@/shutdown/Shutdown.service';
export const LOWEST_PRIORITY = 0;
export const DEFAULT_PRIORITY = 100;
export const HIGHEST_PRIORITY = 200;
import { DEFAULT_SHUTDOWN_PRIORITY } from '@/constants';
/**
* Decorator that registers a method as a shutdown hook. The method will
@ -26,7 +23,7 @@ export const HIGHEST_PRIORITY = 200;
* ```
*/
export const OnShutdown =
(priority = DEFAULT_PRIORITY): MethodDecorator =>
(priority = DEFAULT_SHUTDOWN_PRIORITY): MethodDecorator =>
(prototype, propertyKey, descriptor) => {
const serviceClass = prototype.constructor as ServiceClass;
const methodName = String(propertyKey);

View file

@ -4,7 +4,8 @@ import { Logger } from '@/Logger';
import ioRedis from 'ioredis';
import type { Cluster, RedisOptions } from 'ioredis';
import type { RedisClientType } from './RedisServiceBaseClasses';
import { LOWEST_PRIORITY, OnShutdown } from '@/decorators/OnShutdown';
import { OnShutdown } from '@/decorators/OnShutdown';
import { LOWEST_SHUTDOWN_PRIORITY } from '@/constants';
@Service()
export class RedisClientService {
@ -23,7 +24,7 @@ export class RedisClientService {
return client;
}
@OnShutdown(LOWEST_PRIORITY)
@OnShutdown(LOWEST_SHUTDOWN_PRIORITY)
disconnectClients() {
for (const client of this.clients) {
client.disconnect();

View file

@ -2,6 +2,7 @@ import { Container, Service } from 'typedi';
import { ApplicationError, ErrorReporterProxy, assert } from 'n8n-workflow';
import type { Class } from 'n8n-core';
import { Logger } from '@/Logger';
import { LOWEST_SHUTDOWN_PRIORITY, HIGHEST_SHUTDOWN_PRIORITY } from '@/constants';
type HandlerFn = () => Promise<void> | void;
export type ServiceClass = Class<Record<string, HandlerFn>>;
@ -33,6 +34,13 @@ export class ShutdownService {
/** Registers given listener to be notified when the application is shutting down */
register(priority: number, handler: ShutdownHandler) {
if (priority < LOWEST_SHUTDOWN_PRIORITY || priority > HIGHEST_SHUTDOWN_PRIORITY) {
throw new ApplicationError(
`Invalid shutdown priority. Please set it between ${LOWEST_SHUTDOWN_PRIORITY} and ${HIGHEST_SHUTDOWN_PRIORITY}.`,
{ extra: { priority } },
);
}
if (!this.handlersByPriority[priority]) {
this.handlersByPriority[priority] = [];
}

View file

@ -73,4 +73,26 @@ describe('OnShutdown', () => {
new TestClass();
}).toThrow('TestClass.onShutdown() must be a method on TestClass to use "OnShutdown"');
});
it('should throw if the priority is invalid', () => {
expect(() => {
@Service()
class TestClass {
@OnShutdown(201)
async onShutdown() {}
}
new TestClass();
}).toThrow('Invalid shutdown priority. Please set it between 0 and 200.');
expect(() => {
@Service()
class TestClass {
@OnShutdown(-1)
async onShutdown() {}
}
new TestClass();
}).toThrow('Invalid shutdown priority. Please set it between 0 and 200.');
});
});