waitUntilReady never throws an error

[fyodorvi]

Simple use case:

ldclient.waitUntilReady().then(() => {
 server.listen();
}).catch((error) => {
 console.error(error);
 process.exit(1);
})

When the client gets 401 error it stops trying to connect, which effectively means that initialisation failed, so I expect waitUntilReady to fail so I can fail to start my server and kill the app, but that never happens (although it logs the error).

[eli-darkly]

This is a reasonable request - we are just trying to decide on the best way to implement it that will preserve backward compatibility.

The issue is this: an unhandled promise rejection is a serious problem that can crash the application. And in the past, users of the SDK have not been required to attach an error handler to this promise, because we never rejected it. So if we start triggering rejections, that is potentially a breaking change. I know that in your case, you want to kill the app anyway if there is a startup error from LD, but that is not necessarily true of everyone - some people may want the app to run with limited functionality, using the default values that the client returns when it's in this state.

Another option would be that we fire the ready event / resolve the promise, just like we would if the client successfully started up, but provide the error as an optional parameter. I know it is a bit odd to receive an error in your then() block, but conceptually the client still becomes "ready" in this case, in the sense that you no longer need to wait for it to do any initialization. (You can also detect that the client is in a failed state by checking its initialized() method.) However, this could also be a breaking change in that some applications may not want to use the client in a failed state.

[eli-darkly]

To avoid breaking existing applications, we're going to implement this by providing a new method, waitForInitialization, that behaves like waitUntilReady except that its promise will be rejected if the client fails. There will also be a new event, "failed", which will be fired instead of "ready" in that case. These changes will be released shortly.

[scottcorgan]

That works great too.

But, why not just a major version bump if it's a breaking change?

[eli-darkly]

@scottcorgan, even in a major version release, we would rather not introduce breaking changes that are as subtle as this one would be. That is, since the success case would still work as before, a developer who didn't read every word of the changelog could easily conclude that the application was still working fine so no code changes were needed - until the first time there was an outage at startup time, which would crash the application. So we feel it is more developer-friendly to add a new method, and deprecate the old one until the next major version release.

[scottcorgan]

That's fair. A new method is an elegant solution.

[eli-darkly]

This is now implemented in version 5.1.0: the new method waitForInitialization behaves like waitUntilReady, but will reject the promise if initialization fails.