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

Custom Aggregator does not receive downstream responses. #1287

Closed
MJeorrett opened this issue Jul 13, 2020 · 3 comments · Fixed by #1826
Closed

Custom Aggregator does not receive downstream responses. #1287

MJeorrett opened this issue Jul 13, 2020 · 3 comments · Fixed by #1826
Assignees
Labels
bug Identified as a potential bug Jan'24 January 2024 release merged Issue has been merged to dev and is waiting for the next release
Milestone

Comments

@MJeorrett
Copy link

MJeorrett commented Jul 13, 2020

Expected Behavior

The content of the responses is passed to the custom aggregator with 200 status codes.

Actual Behavior

Responses passed to implementation of the IDefinedAggregator are 404 and their bodies are empty (despite the actual downstream requests apparently succeeding). Perhaps I am missing something blatantly obvious but I am stumped. Could be that the documentation just hasn't quite caught up yet as this extension point seems to have drastically changed recently as noted in this issue: #1069.

Steps to Reproduce the Problem

I have created a minimal repro here: https://github.com/MJeorrett/OcelotAggregationIssue. It contains 2 self hosted aspnet core servers, running on localhost:5201 and localhost:5202, and an server running ocelot (including an aggregated edpoint) on localhost:5200.

Note: *If i downgrade the project to version 14.1.3 of Ocelot, the custom aggregator is passed the successful responses as expected, see this branch in the linked repo.

ocelot.json configures aggregate endpoint like so:

{
  "Routes": [
    {
      "DownstreamPathTemplate": "/downstream1",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "localhost",
          "Port": 5201
        }
      ],
      "UpstreamPathTemplate": "/api/downstream1",
      "UpstreamHttpMethod": [ "Get" ],
      "Key": "downstream1"
    },
    {
      "DownstreamPathTemplate": "/downstream2",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "localhost",
          "Port": 5202
        }
      ],
      "UpstreamPathTemplate": "/api/downstream2",
      "UpstreamHttpMethod": [ "Get" ],
      "Key": "downstream2"
    }
  ],
  "Aggregates": [
    {
      "RouteKeys": [
        "downstream1",
        "downstream2"
      ],
      "UpstreamPathTemplate": "/api/aggregate",
      "Aggregator": "MyAggregator"
    }
  ],
  "GlobalConfiguration": {
    "BaseUrl": "http://localhost:5200"
  }
}

MyAggregator is implemented like so:

public class MyAggregator : IDefinedAggregator
    {
        // Inject _logger in constructor

        public async Task<DownstreamResponse> Aggregate(List<HttpContext> responses)
        {
            var responseBodies = responses.Select(response =>
            {
                _logger.LogDebug($"Status code {response.Response.StatusCode}.");

                using var responseReader = new StreamReader(response.Response.Body);
                return responseReader.ReadToEnd();
            }).ToList();

            return new DownstreamResponse(
                new StringContent(JsonConvert.SerializeObject(responseBodies)),
                HttpStatusCode.OK,
                new List<Header>(),
                "OK");
        }
    }

If you look at the logs from the OcelotAggregation project it seems that the downstream requests succeed but the response passed to my custom aggregator are 404:

dbug: Ocelot.DownstreamUrlCreator.Middleware.DownstreamUrlCreatorMiddleware[0]
      requestId: 0HM177E53QAI3:00000001, previousRequestId: no previous request id, message: Downstream url is http://localhost:5202/downstream2
info: Ocelot.Requester.Middleware.HttpRequesterMiddleware[0]
      requestId: 0HM177E53QAI3:00000001, previousRequestId: no previous request id, message: 200 (OK) status code, request uri: http://localhost:5202/downstream2
dbug: Ocelot.Requester.Middleware.HttpRequesterMiddleware[0]
      requestId: 0HM177E53QAI3:00000001, previousRequestId: no previous request id, message: setting http response message
info: Ocelot.Requester.Middleware.HttpRequesterMiddleware[0]
      requestId: 0HM177E53QAI3:00000001, previousRequestId: no previous request id, message: 200 (OK) status code, request uri: http://localhost:5201/downstream1
dbug: Ocelot.Requester.Middleware.HttpRequesterMiddleware[0]
      requestId: 0HM177E53QAI3:00000001, previousRequestId: no previous request id, message: setting http response message
dbug: OcelotAggregation.MyAggregator[0]
      Status code 404.
dbug: OcelotAggregation.MyAggregator[0]
      Status code 404.

Specifications

  • Version: Ocelot v16.0.1; netcoreapp3.1
  • Platform: AspNet Core (self hosted) running on Windows.
  • Subsystem:
@jlukawska
Copy link
Contributor

Hello,
I've looked at the Ocelot tests and I've found that the responses in the custom aggregator are accessed differently.

var one = await responses[0].Items.DownstreamResponse().Content.ReadAsStringAsync();
var two = await responses[1].Items.DownstreamResponse().Content.ReadAsStringAsync();

And it actually works, both in the acceptance tests and in my sample program.

I think the docs should be updated with the sample Custom Aggregator. I'll try to make a PR soon.

@raman-m raman-m added bug Identified as a potential bug accepted Bug or feature would be accepted as a PR or is being worked on labels Aug 4, 2023
@raman-m
Copy link
Member

raman-m commented Aug 4, 2023

The issue has been accepted due to open PR #1330 by @jlukawska :

@raman-m
Copy link
Member

raman-m commented Aug 4, 2023

@MJeorrett commented on Jul 13, 2020

Hi Mett!
Thanks for your interest in Ocelot!

I would say that your description is perfect, the best I've seen in backlog. 🥇


I have created a minimal repro here: https://github.com/MJeorrett/OcelotAggregationIssue. It contains 2 self hosted aspnet core servers, running on localhost:5201 and localhost:5202, and an server running ocelot (including an aggregated endpoint) on localhost:5200.

This is very important to create such mini-repos to be able to reproduce a bug easily.
I always ask the authors to upload the code to GitHub for open bugs.


Matt,
Could you review current open the PR #1330 please?
Thanks!

@raman-m raman-m added the Jan'24 January 2024 release label Mar 1, 2024
@raman-m raman-m added this to the January'24 milestone Mar 1, 2024
raman-m added a commit that referenced this issue Mar 1, 2024
…1826)

* Cleanup of Multiplexing middleware, avoiding creating copies of the Http context if only one downstream route.

* updating comments

* preparing rebase, it's a hack.

* fixing test case "Copy_User_ToTarget",  method name has changed.

* adding some unit tests, checking that if 1 route, ProcessSingleRoute is called and no copies of context are made, if more than 2 Map is called, then more than 2 copies are created.

* Applying refactoring suggested.

* some code cleanup

* Some code cleanup in Aggregate Logic

* Cleanup in Aggregate Tests, verifying multiplexer cleanup

* Finalizing unit test, with complex keys.

* some code refactoring and applying suggestions from reviews

* Update requestaggregation.rst

Recover docs

* updating docs

* Update requestaggregation.rst

* Update requestaggregation.rst

* Code review by @raman-m

* Inherit `Steps` functionality instead of private aggregation

---------

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>
@raman-m raman-m added merged Issue has been merged to dev and is waiting for the next release and removed accepted Bug or feature would be accepted as a PR or is being worked on labels Mar 1, 2024
@raman-m raman-m mentioned this issue Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identified as a potential bug Jan'24 January 2024 release merged Issue has been merged to dev and is waiting for the next release
Projects
None yet
3 participants