Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
2 tasks done
chihabhajji opened this issue Oct 21, 2024 · 6 comments
Open
2 tasks done
Labels

Comments

@chihabhajji
Copy link

chihabhajji commented Oct 21, 2024

📜 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?

  • I checked and didn't find a similar issue

🏢 Have you read the Contributing Guidelines?

Are you willing to submit PR?

Yes I am willing to submit a PR!

Copy link

linear bot commented Oct 21, 2024

@chihabhajji chihabhajji changed the title 🐛 Bug Report: 🐛 Bug Report: nestjs bootstrap fails due swagger being unable to serialize operations Oct 23, 2024
@chihabhajji
Copy link
Author

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

@chihabhajji
Copy link
Author

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
Copy link

@chihabhajji +1 on this issue.

@rifont
Copy link
Contributor

rifont commented Oct 31, 2024

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
Copy link
Author

will attempt to PR if i find time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants