LDClient.flush() does not reject if there is an error sending analytics

[hysmio]

Describe the bug
LDClient.flush() does not reject like it states if the underlying EventProcessor.flush() call throws, irrespective of callback.

if a callback is provided, the callback will receive the error, the promise will not reject as intended. If no callback is provided, the promise will still not reject, which is unintended as per the JSDoc.

To reproduce

1. Call await LDClient.flush() with an explicit eventsUri that has an error status code, like 503.
2. It will not throw an error
3. Call it again but with a callback await LDClient.flush((error, success) => console.error(success, error));, you will get a console error output.

Expected behavior
The promise on await LDClient.flush() should reject if there is an error.

Logs
If applicable, add any log output related to your problem.

SDK version

@launchdarkly/node-server-sdk@9.7.3
@launchdarkly/js-server-sdk-common@2.11.0
@launchdarkly/js-sdk-common@2.13.0

Language version, developer tools

node - v20.11.1

OS/platform

Windows 11
WSL - Ubuntu 24.04

Solution
Added some comments to explain what the solution I think could be, also found that the callback is called with an error and then immediately again without one, so that should also be fixed.

# LDClient.flush
  async flush(callback?: (err: Error | null, res: boolean) => void): Promise<void> {
    try {
      await this._eventProcessor.flush();
    } catch (err) {
      if (!callback) { // If callback isn't set, throw the error to the caller.
        throw err;
      }
      // also this will currently callback twice regardless, there should probably be a return after this call
      callback(err as Error, false);
    }
    callback?.(null, true);
  }
```ts

[kinyoklion]

@hysmio Thank you for the report.

Filed internally as SDK-1070

[kinyoklion]

@hysmio I am interested in the use case for the flush error handling. The callback error needs resolved for sure, and I will be making an issue to handle it correctly.

I am hesitant to actually reject the flush promise. First because it has not been rejected in several years and as a result people are likely not handling it correctly.

Generally speaking manually flushing the client isn't often needed, aside from short running instance (start, do some work, flush and close).

If the flush fails, the events are already gone by that point, and flushing again would either have no events to flush (as they were discarded from the last flush), or would flush new accumulated events since the last time flush was called. Internally the batch of events will be retried once during the process, so for the flush to fail the request must fail twice (if the error was not terminal).

So I mainly want to validate that the thought isn't to retry, as the events would already be gone.

Thank you,
Ryan

[kinyoklion]

The duplicate callback issue has been resolved. Awaiting some feedback on earlier questions before tackling the other issue. My reflex is to change the documentation to indicate it does not throw, and to also elaborate about why retrying will generally not be useful.