domaindrivendev / Swashbuckle.AspNetCore

Swagger tools for documenting API's built on ASP.NET Core
MIT License
5.26k stars 1.32k forks source link

code gen failing due to invoking code that requires live environment #2438

Open Snazzie opened 2 years ago

Snazzie commented 2 years ago

I have a post build entry in csproj to run codegen after build.

    <Target Name="OpenAPI" AfterTargets="Build" Condition="$(Configuration)=='Debug'">
        <Exec Command="dotnet swagger tofile --yaml --output ./Swagger/v1/$(AssemblyName).yaml $(OutputPath)$(AssemblyName).dll v1" WorkingDirectory="$(ProjectDir)" />
        <Exec Command="dotnet swagger tofile --output ./Swagger/v1/swagger.json $(OutputPath)$(AssemblyName).dll v1" WorkingDirectory="$(ProjectDir)" />
    </Target>

My api controllers dependency injects http clients that throw if they're unable to connect to respected endpoints. So my builds are always failing due to the http client failing to connect to my dbs as they're not running at build time.

This means for build and codegen to succeed, any endpoints it depends on needs to be running. (which i think is quite silly as we're in building stage, not running or testing)

The way around this right now is to wrap these clients in try catch blocks. For k8 deployment, you need to add in workaround conditions to throw if running in live, so health probes can do its thing.

Snazzie commented 2 years ago

I failed to mention that my hosted services use these http clients aswell. (builder.Services.AddHostedService) which i believe are also initialized by codegen

Snazzie commented 2 years ago

It also seems this fails to load appsettings into configurations, which causes connection string to be empty. Results in DbContext.Migrate() to fail.

Snazzie commented 2 years ago

It also seems this fails to load appsettings into configurations, which causes connection string to be empty. Results in DbContext.Migrate() to fail.

I found https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/872#issuecomment-418902908 Turns out by default swagger toFile runs in release mode, so its using appsettings not appsettings.development.

ichbinsteffen commented 1 year ago

It also seems this fails to load appsettings into configurations, which causes connection string to be empty. Results in DbContext.Migrate() to fail.

I found #872 (comment) Turns out by default swagger toFile runs in release mode, so its using appsettings not appsettings.development.

How is this not part of the documentation? I've been troubleshooting my build fails for hours. For anyone stumbling upon this: 2023-06-28 #872 still valid.

This is still the only way to generate the swagger file when your asp net app relies on a ConnectionString or other configuration values that are resolved on the basis of the ASPNET_ENVIRONMENT environment variable.

I mean yes it makes sense that the app needs to be started. The OpenApi Document can be modified in the configuration phase at runtime of an ASP NET project, i.e when you use IncludeXmlComments(filePath, includeControllerXmlComments: true), or through Schema filters that you might add to get your enums documented nicely etc. So yes, the application needs to be started in order to obtain the correct OpenAPI document and I was very confused that there is no configuration cli argument available for it.

Snazzie commented 1 year ago

I mean yes it makes sense that the app needs to be started.

If Microsoft can link everything together, provide static analysis and intellisense without starting the application. This should be able to also. If not in its current state, they should work with microsoft to get it to that state.

atomgregg commented 1 year ago

Found this thread after similar issues generating the file.

I agree with @ichbinsteffen , this kind of behaviour should be in the doc's, not in an issue thread.

That said, I can't get any of the different comments/approaches working, and need to ask for your support.

I have the following simple bat script that I manually run whenever I have changes in my API and want to begin working them into my client.

@echo off

echo "building to ensure what we generate is up to date"
dotnet build

echo "generating swagger"
dotnet swagger tofile --output ..\AdminWebClient\api-spec\swagger.json bin\Debug\net7.0\AdminWebApi.dll v1

echo "generating services"
cd ..\AdminWebClient
npm run nswag

The dotnet swagger tofile command triggers my code to try and connect to the live database (startup routine includes a scheduler that reads config from the DB).

I have an appSettings.Development.json and different code checks in place, but these aren't being evaluated because the environment variable is not being respected when I set the value in the script, or when I set it manually at a terminal before running the tofile command, e.g.

set ASPNETCORE_ENVIRONMENT=Development
dotnet swagger tofile --output ..\AdminWebClient\api-spec\swagger.json bin\Debug\net7.0\AdminWebApi.dll v1

Or using the && approach also suggested:

set ASPNETCORE_ENVIRONMENT=Development && dotnet swagger tofile --output ..\AdminWebClient\api-spec\swagger.json bin\Debug\net7.0\AdminWebApi.dll v1

Can I please request some assistance to set the variable such that my API DLL is run with this?

github-actions[bot] commented 7 months ago

This issue is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further updates are made.

martincostello commented 7 months ago

This doesn't seem to be an issue with Swashbuckle itself from what I can see, but more that apps aren't detecting/accounting for them being run at "design time" and doing things they shouldn't in that scenario.

Happy to accept a PR to the README with an appropriate note/example on how this should be achieved by an app. As noted, I would have thought setting environment variables before invoking the tool would achieve this.

martincostello commented 7 months ago

Related: https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/2452#issuecomment-1250130336