novuhq / novu

Open-Source Notification Platform. Embeddable Notification Center, E-mail, Push and Slack Integrations.
https://novu.co
Other
35.47k stars 3.92k forks source link

🐛 Bug Report: nestjs bootstrap fails due swagger being unable to serialize operations #6735

Open chihabhajji opened 1 month ago

chihabhajji commented 1 month ago

📜 Description

Nestjs application fails to bootstrap because of invalid swagger definition

👟 Reproduction steps

i have a minimal reproduction using v2.3.0

my app imports module has

SecretsModule,
        NovuModule.registerAsync({
            imports: [SecretsModule],
            inject: [NovuConfig, SystemConfig],
            useFactory: (novuConfig: NovuConfig, systemConfig: SystemConfig) => ({
                apiPath: '/novu',
                workflows: [],
                client: new Client({
                    secretKey: novuConfig.apiKey,
                    strictAuthentication: systemConfig.isProduction(),
                }),
            }),

in my main.ts i have

 const apiDocsConfig = new DocumentBuilder()
        .setTitle('Finvo API')
        .setDescription('APIs for Finvo')
        .setVersion('1.0.0')
        .addBearerAuth()
        .build();
    function operationIdFactory(controllerKey: string, methodKey: string) {
    const strippedMethodKey = methodKey.replace('reserved', '');
    const strippedControllerKey = controllerKey.replace('Controller', '');
    return `${strippedControllerKey}${capitalize(strippedMethodKey)}`;
}
    SwaggerModule.setup(`docs`, app, SwaggerModule.createDocument(app, apiDocsConfig, { operationIdFactory }), {
        swaggerOptions: {
            operationsSorter: 'alpha',
            tagsSorter: 'alpha',
            persistAuthorization: true,
        },
        customSiteTitle: 'Finvo API',
        yamlDocumentUrl: `/docs/yaml`,
    });

👍 Expected behavior

application to bootstrap, and optionally swagger to have valid documentation

👎 Actual Behavior with Screenshots

image

Novu version

Novu SaaS

npm version

2.3.0

node version

20.18.0

📃 Provide any additional context for the Bug.

No response

👀 Have you spent some time to check if this bug has been raised before?

🏢 Have you read the Contributing Guidelines?

Are you willing to submit PR?

Yes I am willing to submit a PR!

linear[bot] commented 1 month ago

NV-4523 🐛 Bug Report: nestjs bootstrap fails due swagger being unable to serialize operations

chihabhajji commented 1 month ago

edit to add a title, dont forget to change linear title too

chihabhajji commented 1 month ago

did you guys really have to remove the workflow editor from v0? i have a task pending from two weeks ago to add a new notification just to get paid :'(

f-langelier commented 1 month ago

@chihabhajji +1 on this issue.

rifont commented 3 weeks ago

Hi @chihabhajji , thanks for reporting this bug.

We are also using the NovuModule internally for our Novu-Managed Bridge endpoint, which we're dogfooding in preparation for a new Dashboard editor that you can read more about here. This new Workflow Editor experience is planned for release in the coming months.

On the issue topic, we also use @nestjs/swagger and experienced this same issue. Please take a look at our custom NovuBridgeController which applies the ApiExcludeController decorator directly, and NovuModule injection of that controller here for an approach to workaround this issue.

We know this isn't ideal, but unfortunately we've not been able to find an approach yet that enables us apply the ApiExcludeController internally within the @novu/framework package to create compatibility with @nestjs/swagger.

Pull requests to solve this problem in the @novu/framework Nest.controller.ts directly will be greatly appreciated! It's our preference not to add @nestjs/swagger as a direct dependency of Framework - instead, @novu/swagger can be provided as an optional peer dependency as needed by package consumers.

chihabhajji commented 3 weeks ago

will attempt to PR if i find time

juwon8891 commented 2 weeks ago

+1