Default timeout(90s) of downstream requests is broken

huanguolin commented 9 months ago

Expected Behavior

According to the documentation, if QoSOptions is not configured, by default, the timeout value for downstream requests is set to 90 seconds. In our project, using Ocelot 18.0.0 + .NET 6, the runtime behavior aligns with the documentation.

Actual Behavior

When we upgraded to Ocelot 22.0.0 + .NET 8, also without configuring QoSOptions, the downstream requests never timeout.

Specifications

Version: 22.0.0
Platform: .NET 8

I took a brief look at the code and noticed that in version 22.0.0, the default value of TimeoutValue in QoSOptions was changed from 0 to int.MaxValue. The code where the default value of 90 seconds is effective seems to be here when it is set to 0. I'm not sure if my analysis is correct.

raman-m commented 9 months ago

Hi Alvin! Thanks for reporting the bug!

Yes, your analysis is correct! The code is not consistent now because the bug was introduced in the default constructor of the FileQoSOptions class.

Will you open a PR with the fix? It should be pretty short and simple to assign zero in default constructor like this

        public FileQoSOptions()
        {
            DurationOfBreak = 1;
            ExceptionsAllowedBeforeBreaking = 0;
            TimeoutValue = 0; // int.MaxValue bug
        }

huanguolin commented 9 months ago

@raman-m Thank you for your prompt response! I'm happy to contribute to the pull request #1834.

raman-m commented 9 months ago

Released by #1847

ThreeMammals / Ocelot