Update getKeys to respond 404 when /featureflag is requested without /init

[zackcl]

Resolves #1802

[bcb37]

One suggestion.

[bcb37]

experimentUserDoc?.id in that interpolated string will never be anything but undefined. We should probably just throw a more generic message. (same on line 50 below)

[zackcl]

Thanks, I've updated the code so that it logs with experimentUserDoc?.requestedUserId instead of experimentUserDoc?.id which will always be undefined. This is exactly the same as how we log and return 404 in the getAllExperimentConditions function.

However, in the current code, the experimentUserDoc?.requestedUserId is also undefined when the user document is not found. This is due to how the getUserDoc function is currently implemented:

public async getUserDoc(experimentUserId, logger): Promise<RequestedExperimentUser> {
    try {
      const experimentUserDoc = await this.getOriginalUserDoc(experimentUserId, logger);
      if (experimentUserDoc) {
        const userDoc = { ...experimentUserDoc, requestedUserId: experimentUserId };
        logger.info({ message: 'Got the user doc', details: userDoc });
        return userDoc;
      } else {
        return null;
      }
    } catch (error) {
      logger.error({ message: `Error in getting user doc for user => ${experimentUserId}`, error });
      return null;
    }
  }

I believe this should be updated to the following to allow logging experimentUserDoc?.requestedUserId when the user document is not found:

public async getUserDoc(experimentUserId: string, logger: UpgradeLogger): Promise<RequestedExperimentUser | null> {
    try {
      const experimentUserDoc = await this.getOriginalUserDoc(experimentUserId, logger);
      const userDoc = { ...experimentUserDoc, requestedUserId: experimentUserId };
      logger.info({ message: 'Got the user doc', details: userDoc });
      return userDoc;
    } catch (error) {
      logger.error({ message: `Error in getting user doc for user => ${experimentUserId}`, error });
      return null;
    }
  }

I can create a separate PR to apply this change. Please feel free to approve this one if this makes sense.

[zackcl]

I created a PR to update the getUserDoc function : #1809

zack implemented this change