nikcio / Nikcio.UHeadless

The easiest way to go headless in Umbraco using GraphQL
https://nikcio.github.io/Nikcio.UHeadless
MIT License
53 stars 13 forks source link

contentByAbsoluteRoute routingmode CACHE_OR_ROUTING requires baseUrl otherwise crash on 404 #321

Closed karlmacklin closed 4 months ago

karlmacklin commented 4 months ago

Which Nikcio.UHeadless version are you using? For example: 3.2.0

4.2.1

Which Umbraco version are you using? For example: 10.4.0 - don't just write v10

13.3.2

Bug summary

Routingmode CACHE_OR_ROUTING requires baseUrl otherwise you will get a crash when querying contentByAbsoluteRoute on a route that is non-existant.

Steps to reproduce

In Umbraco, make sure your "main node" does not have a set domain under Domain and hostnames.

Then make a query as such:

query PageByContentByAbsoluteRoute {
  contentByAbsoluteRoute(
    route: "/asdf",
    routeMode: CACHE_OR_ROUTING
  ) {
    id
  }
}

Also, compare the query to our current workaround that gives a proper null response:

query PageByContentByAbsoluteRoute {
  contentByAbsoluteRoute(
    baseUrl: "http://hostname"
    route: "/asdf",
    routeMode: CACHE_OR_ROUTING
  ) {
    id
  }
}

Expected result / actual result

Expected result would be the response when including a baseUrl that can be parsed:

{
  "data": {
    "contentByAbsoluteRoute": null
  }
}

Actual result is this error thrown:

{
  "errors": [
    {
      "message": "Unexpected Execution Error",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "contentByAbsoluteRoute"
      ],
      "extensions": {
        "message": "Invalid URI: The format of the URI could not be determined.",
        "stackTrace": "   at System.Uri.CreateThis(String uri, Boolean dontEscape, UriKind uriKind, UriCreationOptions& creationOptions)\r\n   at System.Uri..ctor(String uriString)\r\n   at Nikcio.UHeadless.Content.Router.ContentRouter`2.GetContentByRouting(String route, String baseUrl, String culture, String segment, Nullable`1 fallback)\r\n   at Nikcio.UHeadless.Content.Queries.ContentByAbsoluteRouteQuery`2.ContentByAbsoluteRoute(IContentRouter`2 contentRouter, String route, String baseUrl, String culture, Boolean preview, RouteMode routeMode, String segment, IEnumerable`1 fallback)\r\n   at FABackend.CustomContentQuery.ContentByAbsoluteRoute(IContentRouter`2 contentRouter, String route, String baseUrl, String culture, Boolean preview, RouteMode routeMode, String segment, IEnumerable`1 fallback) in C:\\code\\fa-cms\\FABackend\\uheadless\\CustomContentQuery.cs:line 42\r\n   at HotChocolate.Resolvers.Expressions.ExpressionHelper.AwaitTaskHelper[T](Task`1 task)\r\n   at HotChocolate.Types.Helpers.FieldMiddlewareCompiler.<>c__DisplayClass9_0.<<CreateResolverMiddleware>b__0>d.MoveNext()\r\n--- End of stack trace from previous location ---\r\n   at HotChocolate.Data.Sorting.Expressions.QueryableSortProvider.<>c__DisplayClass9_0`1.<<CreateExecutor>g__ExecuteAsync|1>d.MoveNext()\r\n--- End of stack trace from previous location ---\r\n   at HotChocolate.Data.Filters.Expressions.QueryableFilterProvider.<>c__DisplayClass10_0`1.<<CreateExecutor>g__ExecuteAsync|1>d.MoveNext()\r\n--- End of stack trace from previous location ---\r\n   at HotChocolate.Execution.Processing.Tasks.ResolverTask.ExecuteResolverPipelineAsync(CancellationToken cancellationToken)\r\n   at HotChocolate.Execution.Processing.Tasks.ResolverTask.TryExecuteAsync(CancellationToken cancellationToken)"
      }
    }
  ],
  "data": {
    "contentByAbsoluteRoute": null
  }
}
nikcio commented 4 months ago

Is there any specific reason to not have a domain set in Umbraco?

karlmacklin commented 4 months ago

Is there any specific reason to not have a domain set in Umbraco?

Not really, but we didn't have that in this case and I figured that if the reproduction steps deviate too much from our setup it may not be representative and might miss triggering the error.

However I just tested this now and I was wrong on that end, even with a domain set it still throws the error.

nikcio commented 4 months ago

From my quick look it seems to break here: https://github.com/nikcio/Nikcio.UHeadless/blob/3fb9403343beed46bd2d8750777a69879a3d6c44/src/Nikcio.UHeadless.Content/Router/ContentRouter.cs#L76

The IPublishedRouter from Umbraco requires a URI so that is the limitation we'll have to follow.

We could add CACHE_OR_ROUTING here then the BaseUrl will be set to the current domain: https://github.com/nikcio/Nikcio.UHeadless/blob/3fb9403343beed46bd2d8750777a69879a3d6c44/src/Nikcio.UHeadless.Content/Queries/ContentByAbsoluteRouteQuery.cs#L48

Is this for a solution where the Backoffice and website are on the same domain?

nikcio commented 4 months ago

However I just tested this now and I was wrong on that end, even with a domain set it still throws the error.

What do you mean by this?

Did you add the domain to Umbraco and the query or just Umbraco?

karlmacklin commented 4 months ago

However I just tested this now and I was wrong on that end, even with a domain set it still throws the error.

What do you mean by this?

Did you add the domain to Umbraco and the query or just Umbraco?

Sorry for the ambiguity - I just added it inside Umbraco backoffice, under Cultures and hostnames.

karlmacklin commented 4 months ago

From my quick look it seems to break here:

https://github.com/nikcio/Nikcio.UHeadless/blob/3fb9403343beed46bd2d8750777a69879a3d6c44/src/Nikcio.UHeadless.Content/Router/ContentRouter.cs#L76

The IPublishedRouter from Umbraco requires a URI so that is the limitation we'll have to follow.

We could add CACHE_OR_ROUTING here then the BaseUrl will be set to the current domain:

https://github.com/nikcio/Nikcio.UHeadless/blob/3fb9403343beed46bd2d8750777a69879a3d6c44/src/Nikcio.UHeadless.Content/Queries/ContentByAbsoluteRouteQuery.cs#L48

Is this for a solution where the Backoffice and website are on the same domain?

Separate domains in this case.

We can work around this quite easily by just making sure to supply a baseUrl for all requests, and we can also manage this in multiple environments by using env vars for test/staging environments etc. So the issue is fairly easy to workaround.

I suppose there's a good argument for needing a baseUrl, but it took us a while to figure out we were crashing for this reason, so in that sense it felt like a bug.

nikcio commented 4 months ago

I can add a guard with a more appropriate error message but in general I think the most correct solution is to always set a domain in Umbraco and supply that domain in the query under baseUrl. This will also give your editors the best experience as the links under the "Info" tab will show the correct links.

You will also be better prepared if the Umbraco instance some day will support multiple sites or just multiple domains for cultures.

But if you want to avoid this for some reason you can just use the CACHE_ONLY mode as this won't require a domain to be fetched.

karlmacklin commented 4 months ago

I think you're right, I'll close this issue 👍🏼