web-sdk: onContextChange not called for named provider

[thomaspoignant]

Observed behavior

I am not sure to understand how do you change the context for a named provider?

Let's say I am doing something like this :

await OpenFeature.setContext({targetingKey: 'foo'});
OpenFeature.setProvider('my-provider', provider);
const client = await OpenFeature.getClient('my-provider');
//...
await OpenFeature.setContext({targetingKey: 'bar'});

Expected Behavior

I was expecting the function onContextChange of the provider to be called when changing the context.
But when looking at the code

js-sdk/packages/client/src/open-feature.ts

Lines 37 to 41 in 5725194

	async setContext(context: EvaluationContext): Promise<void> {
	const oldContext = this._context;
	this._context = context;
	await this._defaultProvider?.onContextChange?.(oldContext, context);
	}

it seems that this function is called only for the default provider.

Is it something expected? Did I miss something to be sure that onContextChange is call on the provider.

[beeme1mr]

Hey @thomaspoignant, I would also expect the SDK to call onContextChange on all registered providers. Looking at the code, it looks like an error would bubble up to the user if the context change method throws in a provider. Is that the functionality we want? @toddbaert, what do you think?

[toddbaert]

Ya I agree this is a bug. Context is global in the static context, that's a clear case. I think this is just a bug that we missed when implementing the js-sdk.

[lukas-reining]

I would start to implement this now.
Should this already be covering the whole static context spec change? I think at the first look it seems that there is not much to do for the spec update anyways.
What do you think @toddbaert @beeme1mr @thomaspoignant?

[beeme1mr]

Hey @lukas-reining, yes, it should cover the static context change. That spec change should be merged today 😄

[lukas-reining]

Looking at the code, it looks like an error would bubble up to the user if the context change method throws in a provider. Is that the functionality we want? @toddbaert, what do you think?

What do we do about this? @beeme1mr