AppConfig "empty" response handling
chrisclayson opened this issue · 6 comments
What were you trying to accomplish?
Using the AppConfigProvider
for the parameters project, I noticed a bug in the handling of instances where AppConfig returns the empty response (i.e. where config has not changed).
This was specifically for a "Freeform configuration profile" rather than feature flags (which I have not tested).
Expected Behavior
The provider should re-use the cached config.
Current Behavior
The provider returns an empty string for the config rather than the cached config. The seems to be because the provider only checks for null
and not "empty".
Possible Solution
I implemented a copy of the AppConfigProvider
class and added a check for response.configuration().asByteArray().length > 0
and this worked, i.e.:
// Get the value of the key. Note that AppConfig will return null if the value
// has not changed since we last asked for it in this session - in this case
// we return the value we stashed at last request.
String value = response.configuration() != null && response.configuration().asByteArray().length > 0 ?
response.configuration().asUtf8String() : // if we have a new value, use it
establishedSession != null ?
establishedSession.lastConfigurationValue :
// if we don't but we have a previous value, use that
null; // otherwise we've got no value
// Update the cache so we can get the next value later
establishedSessions.put(key, new EstablishedSession(nextSessionToken, value));
return value;
I'm happy to submit as a PR.
Steps to Reproduce (for bugs)
- Setup a freeform configuration profile in AppConfig with some value (we used a JSON object
{ "version": 1 }
) - Retrieve from a lambda using PowerTools parameters:
var config = ParamManager.getAppConfigProvider("test", "test")
.withMaxAge(30, ChronoUnit.SECONDS)
.get("test");
- Fire continuous traffic at the lambda
The first retrieval, on any given runtime environment, will work fine, but after the 30 second TTL the next retrieve from AppConfg returns an empty string, which is returned from the get()
method rather than original (cached value).
Environment
- Powertools for AWS Lambda (Java) 1.18.0
- Built with Gradle
- Java 17 runtime
Thank you @chrisclayson for submitting this. And you're more than welcome to create a PR.
@scottgerring, can you have a look?
I'll have to fork to submit PR, so I'll give that a go.
One thing I forgot to mention is, initially, this manifest itself with this error here ... I haven't fully worked out why, but we use a transformer, and I don't think it was handling the empty (or null
) config being passed to it very well but just confused as to why it triggered that particular error.
I'll raise as a separate bug if I think it one for you to worry about ... it may have been my ropey transformer code.
Wondering if there is any plans to either fix the issue or merge my PR in (I note it failed a bunch of checks, which I'm assuming because from a fork)?
I have a use case for this and, whilst I can supply my own provider with the fix, I'd rather use a supported "out of the box" version ;-)
@jeromevdl @chrisclayson apologies
for the slow response!
Fix looks about right to me - maybe something like StringUtils.isNullOrEmpty
is convenient?
Afternoon, I see this has been tagged for the next release ... just wondering if there is a timeline for the release?