search

imazen / imageflow-dotnet-server

A super-fast image server to speed up your site - deploy as a microservice, serverless, or embeddable.
https://docs.imageflow.io
GNU Affero General Public License v3.0
252 stars 33 forks source link

Add error handling #32

Closed edmacdonald closed 3 years ago

edmacdonald commented 3 years ago

This will address some of the mysterious errors as reported here: https://github.com/imazen/imageflow-dotnet-server/issues/31#issue-744393915

For remotes that return 404's the RemoteReader was happily passing on the html page.

lilith commented 3 years ago

If I understand properly, calling GetClient() just once rather than on-demand can prevent DNS updates from propagating.

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-5.0#consumption-patterns

It also looks like cookies could end up being shared across multiple requests unless that is blocked in configuration.

lilith commented 3 years ago

I think we need to spend a bit more time looking at the implementation details and try to get everything right in one breaking change rather than in multiple.

lilith commented 3 years ago

Have you looked into using Polly?

edmacdonald commented 3 years ago

What I really wanted to do was introduce the error handling, I should have resisted the urge to change the http client. I've backed that stuff out, and will spend more time looking into that.

I looked briefly at Polly, but didn't want to introduce a third-party dependency. If you're OK with that, I'll look into that as well.

lilith commented 3 years ago

You're definitely headed in the right direction with using the factory instead but you're right we need to make that a separate PR for the breakage.

I am ok with adding dependencies to make it mor robust.

On Fri, Nov 20, 2020, 6:41 AM Ed MacDonald notifications@github.com wrote:

What I really wanted to do was introduce the error handling, I should have resisted the urge to change the http client. I've backed that stuff out, and will spend more time looking into that.

I looked briefly at Polly, but didn't want to introduce a third-party dependency. If you're OK with that, I'll look into that as well.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow-dotnet-server/pull/32#issuecomment-731176444, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2LH5QBJ4UWIS243BTZO3SQZWX5ANCNFSM4T4DYB6A .