Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Consider adding callback for load or reload errors for all providers #572

Closed
ycrumeyrolle opened this issue Dec 21, 2016 · 6 comments
Closed

Comments

@ycrumeyrolle
Copy link

Extension of the issue #489.
As for now, it is possible to handle exception occurring on load only for file-based providers.

            builder.AddJsonFile("settings.json")
                   .SetFileLoadExceptionHandler(ctx =>
                   {
                       // handle error
                   });

Exceptions may occurs with every provider, especially with remote providers like Azure Key Vault & Redis providers. The SetFileLoadExceptionHandler() extension method should be replaced by a more generic SetLoadExceptionHandler() method.

Question : Can the handler be generic enough for been common to all providers ?
Or should it be specific for each provider and defined in the AddXxx() method ?

@divega
Copy link

divega commented Dec 21, 2016

This makes sense to me. If we add this more general purpose extension I think we should discuss in detail how we make the file-based providers converge to it and how much compatibility we need to maintain.

I think it is possible to make the pattern more generic by changing some of the property types on https://github.com/aspnet/Configuration/blob/dev/src/Microsoft.Extensions.Configuration.FileExtensions/FileLoadExceptionContext.cs. Hopefully we can figure a way so that usability for file-based providers is not affected.

@divega
Copy link

divega commented Dec 21, 2016

cc @HaoK @glennc

@HaoK
Copy link
Member

HaoK commented Dec 21, 2016

We can certainly push it into base, but then it needs to go onto then base ConfigurationProviderSource as well

@HaoK
Copy link
Member

HaoK commented Dec 21, 2016

We'd probably also have to change the current ConfigurationProvider Load() to be non virtual and introduce another virtual load to automatically hook this up

@HaoK
Copy link
Member

HaoK commented Jan 4, 2017

@divega do you want me to take a look at this for 2.0?

@divega divega added this to the 2.0.0 milestone Jan 4, 2017
@divega
Copy link

divega commented Jan 4, 2017

@HaoK sounds good. cc @glennc

@ajcvickers ajcvickers modified the milestones: 2.0.0-preview1, 2.0.0 Apr 19, 2017
@shirhatti shirhatti modified the milestones: Backlog, 2.0.0 May 16, 2017
@HaoK HaoK removed the 2 - Working label Jun 2, 2017
@HaoK HaoK removed their assignment Jun 2, 2017
@ajcvickers ajcvickers removed this from the Backlog milestone Dec 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants