Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onError not firing when no envrionment id is specified #75

Closed
sammysium opened this issue Jun 8, 2021 · 2 comments
Closed

onError not firing when no envrionment id is specified #75

sammysium opened this issue Jun 8, 2021 · 2 comments
Assignees
Labels
help wanted Extra attention is needed

Comments

@sammysium
Copy link

Perhaps I am wrong but I was under the impression that when there is an error initalizing flagsmith, the onError call back is called. Here is my step, which on purpose doesn't have envrionment Id.

handleFlagsError has logic depending on the envrionment that I want to confirm with a unit test. The following is the code:

const handleFlagsError = (e) => {
  console.log('reached error handler');
};

const initializeFlagSmith = () => {
   flagsmith.init({
    environmentID: Config.FS_ENVID,
    api: 'url',
    asyncStorage: AsyncStorage,
    cacheFlags: true,
    onError: handleFlagsError,
    defaultFlags: TabletConfigurations
  });
  flagsmith.startListening(30000);
};

now I understand envrionment ID is required but i am not seeing logs in my console. Now in my test I am doing:

Config.FS_ENVID = undefined;
initializeFlagSmith();

I don't see console from handleFlagsError but I do see Please specify a environment id which is thrown out I believe by flagsmith itself. I was hoping to see simply:

'reached error handler'

hope I made sense.

@dabeeeenster dabeeeenster added the help wanted Extra attention is needed label Jun 8, 2021
@kyle-ssg
Copy link
Member

kyle-ssg commented Jun 9, 2021

Hey @sammysium, sure that makes sense. I'll look at having it trigger the onError too, this was originally intended for errors during flag retrieval but I see your point.

@sammysium
Copy link
Author

the fix seems good enough for now @kyle-ssg thanks so much.

kyle-ssg added a commit that referenced this issue Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants