CarterCommunity / Carter

Carter is framework that is a thin layer of extension methods and functionality over ASP.NET Core allowing code to be more explicit and most importantly more enjoyable.
MIT License
2.08k stars 175 forks source link

OpenApi - Calling '/openapi' throws an error (NetCoreApp3.0). #200

Closed stephenpope closed 4 years ago

stephenpope commented 5 years ago

System Info :

.NET Core SDK (reflecting any global.json):
 Version:   3.0.100-preview6-012264
 Commit:    be3f0c1a03

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  10.14
 OS Platform: Darwin
 RID:         osx.10.14-x64
 Base Path:   /usr/local/share/dotnet/sdk/3.0.100-preview6-012264/

Basic web app..

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp3.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Carter" Version="4.2.0" />
  </ItemGroup>

</Project>

Simple module ..

    public class HomeModule : CarterModule
    {
        public HomeModule()
        {
            Get<GetDefault>("/", async (req, res, routeData) =>
            {
                res.StatusCode = 409;
                await res.WriteAsync("There's no place like 127.0.0.1");
            });
        }
    }

    public class GetDefault : RouteMetaData
    {
        public override string Description { get; } = "A description.";

        public override RouteMetaDataResponse[] Responses { get; } =
        {
            new RouteMetaDataResponse
            {
                Code = 200,
                Description = "A response description.",
                Response = typeof(string)
            }
        };

        public override string Tag { get; } = "Basic";
    }

Calling https://localhost:5001/openapi errors with ..

An unhandled exception has occurred while executing the request.
System.NullReferenceException: Object reference not set to an instance of an object.
   at Carter.OpenApi.CarterOpenApi.<>c__DisplayClass0_0.<BuildOpenApiResponse>b__0(HttpContext context)
   at Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)
jchannon commented 5 years ago

Is it just netcore 3 SDK, works ok on 2.x?

On Thu, 18 Jul 2019 at 16:21, Stephen Pope notifications@github.com wrote:

System Info :

.NET Core SDK (reflecting any global.json): Version: 3.0.100-preview6-012264 Commit: be3f0c1a03

Runtime Environment: OS Name: Mac OS X OS Version: 10.14 OS Platform: Darwin RID: osx.10.14-x64 Base Path: /usr/local/share/dotnet/sdk/3.0.100-preview6-012264/

Basic web app..

netcoreapp3.0

Simple module ..

public class HomeModule : CarterModule
{
    public HomeModule()
    {
        Get<GetDefault>("/", async (req, res, routeData) =>
        {
            res.StatusCode = 409;
            await res.WriteAsync("There's no place like 127.0.0.1");
        });
    }
}

public class GetDefault : RouteMetaData
{
    public override string Description { get; } = "A description.";

    public override RouteMetaDataResponse[] Responses { get; } =
    {
        new RouteMetaDataResponse
        {
            Code = 200,
            Description = "A response description.",
            Response = typeof(string)
        }
    };

    public override string Tag { get; } = "Basic";
}

Calling https://localhost:5001/openapi errors with ..

An unhandled exception has occurred while executing the request. System.NullReferenceException: Object reference not set to an instance of an object. at Carter.OpenApi.CarterOpenApi.<>c__DisplayClass0_0.b__0(HttpContext context) at Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext) at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/200?email_source=notifications&email_token=AAAZVJRHBX2DSG54ZCWZO53QACC7ZA5CNFSM4IE4ZNCKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HABC5YA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZVJRYHZ3WT55TWE4IL23QACC7ZANCNFSM4IE4ZNCA .

stephenpope commented 5 years ago

Same error when running as NetcoreApp2.2 (still with NetCore 3 SDK installed). Have I done something wrong in my setup ?

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.2</TargetFramework>
    <AspNetCoreHostingModel>InProcess</AspNetCoreHostingModel>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Carter" Version="4.2.0" />
    <PackageReference Include="Microsoft.AspNetCore.App" />
    <PackageReference Include="Microsoft.AspNetCore.Razor.Design" Version="2.2.0" PrivateAssets="All" />
  </ItemGroup>

</Project>

using ..

{
  "sdk": {
    "version": "2.2.300"
  }
}

Error:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Carter.OpenApi.CarterOpenApi.<>c__DisplayClass0_0.<BuildOpenApiResponse>b__0(HttpContext context) in /Users/jonathan/Projects/Carter/src/OpenApi/CarterOpenApi.cs:line 23
   at Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)
jchannon commented 5 years ago

Seems ok,

Only thing I notice is netcore 3 sdk, sdk in csproj i tend to use is withouth the .Web suffix, I don't tend to use Microsoft.AspNetCore.App dependency either

Might be worth seeing if you can find the difference with the sample project as that does work https://github.com/CarterCommunity/Carter/tree/master/samples/CarterSample

On Thu, 18 Jul 2019 at 16:38, Stephen Pope notifications@github.com wrote:

Same error when running as NetcoreApp2.2 (still with NetCore 3 SDK installed). Have I done something wrong in my setup ?

netcoreapp2.2 InProcess

using ..

{ "sdk": { "version": "2.2.300" } }

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/200?email_source=notifications&email_token=AAAZVJQXJYG3VVJTKT3S3JLQACFAFA5CNFSM4IE4ZNCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2I4OBY#issuecomment-512870151, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZVJWYF3UTCZRAIMY3Q33QACFAFANCNFSM4IE4ZNCA .

stephenpope commented 5 years ago

The sample works ok. I will work backwards from there. It says its around L23 so it might be because I have not specified any OpenApiOptions but I will check now.

stephenpope commented 5 years ago

Right .. so I added some CarterOptions ..

private CarterOptions GetOptions(ICollection<string> addresses)
        {
            return new CarterOptions(openApiOptions:
                new OpenApiOptions(documentTitle: "Carter <3 OpenApi", addresses: addresses, new Dictionary<string, OpenApiSecurity>())); 
        }

.. which fixed my error (Im assuming its the OpenApiOptions or documentTitle being null thats causing the failure.) but then i ran smack into the Synchronous operation not allowed error (https://github.com/CarterCommunity/Carter/issues/39) ... so had to add AllowSynchronousIO into my Program.cs:

public static IHostBuilder CreateHostBuilder(string[] args) =>
  Host.CreateDefaultBuilder(args).ConfigureWebHostDefaults(webBuilder =>
  {
      webBuilder.UseStartup<Startup>().ConfigureKestrel(((context, options) =>
      {
          options.AllowSynchronousIO = true;
      }));
});
jchannon commented 4 years ago

Soooooo, just merged lots of OpenApi changes and OpenApiOptions needs supplying if you want openapi to work. Question is how to enforce that or do we leave it as is.....

jussimattila commented 4 years ago

I think requiring OpenApiOptions for the functionality is fine. Just need to update the docs, as it currently says:

Carter supports OpenApi out of the box. Simply call /openapi from your API and you will get a OpenApi JSON response.

An exception that explicitly mentions the requirement would also be useful, if one fails to supply options.

jchannon commented 4 years ago

That should still be the case?

On Tue, 26 Nov 2019 at 08:23, Jussi Mattila notifications@github.com wrote:

I think requiring OpenApiOptions for the functionality is fine. Just need to update the docs, as it currently says:

Carter supports OpenApi out of the box. Simply call /openapi from your API and you will get a OpenApi JSON response.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/200?email_source=notifications&email_token=AAAZVJQCQSWBAEJL72E3N63QVTMIXA5CNFSM4IE4ZNCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFFESXI#issuecomment-558516573, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJQSYK6CEJOTOSOC4VTQVTMIXANCNFSM4IE4ZNCA .

stephenpope commented 4 years ago

Agreeing with @jussimattila - Fine as long as the exception is more helpful than the previous one :)

jussimattila commented 4 years ago

Yeah, instead of System.NullReferenceException: Object reference not set to an instance of an object. one should see InvalidOperationException: OpenApiOptions have not been provided to Carter.

jchannon commented 4 years ago

Just tested it with services.AddCarter() and app.UseEndpoints(builder => builder.MapCarter()); and called /openapi and no exception.

On Tue, 26 Nov 2019 at 08:27, Jussi Mattila notifications@github.com wrote:

Yeah, instead of System.NullReferenceException: Object reference not set to an instance of an object. one should see `InvalidOperationException: OpenApiOptions have not been provided to Carter."

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

jussimattila commented 4 years ago

I recently setup openapi with Carter v4.2.0. That gave the null reference error, which brought me here. Thanks @stephenpope for the fix, BTW. And sorry @jchannon for not testing with the latest bits!

jchannon commented 4 years ago

Ah no worries, I did a release last night and thought it was related :)

On Tue, 26 Nov 2019 at 08:34, Jussi Mattila notifications@github.com wrote:

I recently setup openapi with Carter v4.2.0. That gave the null reference error, which brought me here. Thanks @stephenpope https://github.com/stephenpope for the fix, BTW. And sorry @jchannon https://github.com/jchannon for not testing with the latest bits!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/200?email_source=notifications&email_token=AAAZVJR6F24RZ5M7WPQK55LQVTNRTA5CNFSM4IE4ZNCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFFFUKY#issuecomment-558520875, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJUK4AFWBJW4WAAVN5LQVTNRTANCNFSM4IE4ZNCA .

stephenpope commented 4 years ago

Does the call the /openapi return anything when no OpenApiOptions are specified? Glad it doesn't NullRef throw but would you want to debug log the issue or something to help nudge people towards what they are missing ? (Sorry if this seems pedantic ;)

jchannon commented 4 years ago

It will still return the openapi json if no options are available but some properties like the title and server urls will be a default value

On Tue, 26 Nov 2019 at 09:13, Stephen Pope notifications@github.com wrote:

Does the call the /openapi return anything when no OpenApiOptions are specified? Glad it doesn't NullRef throw but would you want to debug log the issue or something to help nudge people towards what they are missing ? (Sorry if this seems pedantic ;)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/200?email_source=notifications&email_token=AAAZVJX5W7BIPIJ4AEJ3CXTQVTSCJA5CNFSM4IE4ZNCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFFJHGQ#issuecomment-558535578, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJS2JUDOZ2OMILCEH4TQVTSCJANCNFSM4IE4ZNCA .

stephenpope commented 4 years ago

Perfect :) Thanks! Happy to close the issue then :)

JoeStead commented 4 years ago

:+1: glad it got sorted