Skip to content
This repository has been archived by the owner on Oct 29, 2024. It is now read-only.

NPE on getLastFailedConnection since it do null.intValue() #160

Closed
dprevost-LMI opened this issue Oct 14, 2022 · 8 comments · Fixed by #224
Closed

NPE on getLastFailedConnection since it do null.intValue() #160

dprevost-LMI opened this issue Oct 14, 2022 · 8 comments · Fixed by #224

Comments

@dprevost-LMI
Copy link

Is this a support request?
I guess not

Describe the bug
We are having some inconsistent issues where the allFlags return an empty object and to get more information, I tried to use the getLastFailedConnection to see if there is any failure. Still, I guess there is not since it is doing an NPE.

Since getLastFailedConnection() is null clearly the below will be a NPE....

promise.resolve(LDClient.getForMobileKey(environment).getConnectionInformation().getLastFailedConnection().intValue());

To reproduce
Call getLastFailedConnection without any failure you will get a NPE

Expected behavior
Return null as mentioned in the doc

        /**
         * Returns the most recent successful flag cache update in millis from the epoch
         * or null if flags have never been retrieved.
         *
         * @param environment
         *   Optional string to execute the function in a different environment than the default.
         * @returns 
         *   A promise containing a number representing the status of the connection to LaunchDarkly, 
         *   or null if a successful connection has yet to be established.
         */
        getLastSuccessfulConnection(environment?: string): Promise<number | null>;

Logs

 Attempt to invoke virtual method 'int java.lang.Long.intValue()' on a null object reference

SDK version
The version of this SDK that you are using.

Language version, developer tools
React native

OS/platform
Mac

Additional context

@louis-launchdarkly
Copy link
Contributor

Hello @dprevost-LMI, thank you for reporting the issue you see.

Given the logs said java, can you confirm that the issue is happening for android? If you use iOS, do you see this issue also for iOS?

Filed internally as 173523.

@louis-launchdarkly
Copy link
Contributor

I don't think that part of the code had changed so we will need to test this for 7.x and see what is the current state.

@dprevost-LMI
Copy link
Author

Hello @dprevost-LMI, thank you for reporting the issue you see.

Given the logs said java, can you confirm that the issue is happening for android? If you use iOS, do you see this issue also for iOS?

Filed internally as 173523.

Yes, it was for Android. I did not test for iOS, but by code inspection, you can easily find that a null not checked can result in a NPE

@louis-launchdarkly
Copy link
Contributor

Hello @dprevost-LMI, do you mind letting me know which version of the SDK you are using? The Exception handling for that method is updated in the RN SDK 7.x - the behavior is now different.

The Type doc is still not accurate though, in 7.x, when there is no failure for getLastFailedConnection (or success for getLastSuccessfulConnection in the document you quoted), it is returning a resolved promise with value 0 instead of null.

@louis-launchdarkly louis-launchdarkly added the waiting for feedback Indicates LaunchDarkly is waiting for customer feedback before issue is closed due to staleness. label May 12, 2023
@dprevost-LMI
Copy link
Author

dprevost-LMI commented May 18, 2023

Hey!

As you can see in this code, the lastFailedConnnection can be set to null.

So it would be just natural to do a null check in that code

So the fix would be:

const lastFailedConnection  = LDClient.getForMobileKey(environment).getConnectionInformation().getLastFailedConnection();
promise.resolve(lastFailedConnection == null ? null: lastFailedConnection.intValue() );

Or, as you said, change the doc there

Note: you have the same problem for LastSuccessfulConnection which I guess does not happen often

@dprevost-LMI
Copy link
Author

@louis-launchdarkly you need more from me?

@louis-launchdarkly louis-launchdarkly removed the waiting for feedback Indicates LaunchDarkly is waiting for customer feedback before issue is closed due to staleness. label Jun 9, 2023
@louis-launchdarkly
Copy link
Contributor

Hello @dprevost-LMI, thank you for the reply - sorry that there is an issue about the workflow (which should have remove the waiting for feedback label). I will discuss with the JS engineer on what is the behavior the SDK should do.

@yusinto
Copy link
Contributor

yusinto commented Jun 12, 2023

@dprevost-LMI we are fixing the null error and will release a new version soon. Thank you for reporting this.

yusinto added a commit that referenced this issue Jun 22, 2023
## [7.1.6] - 2023-06-22
### Fixed:
- Fix #160 null pointer exceptions on `getLastSuccessfulConnection` and
`getLastFailedConnection`

---------

Co-authored-by: Ember Stevens <ember.stevens@launchdarkly.com>
Co-authored-by: Ember Stevens <79482775+ember-stevens@users.noreply.github.com>
Co-authored-by: Yusinto Ngadiman <yusinto@gmail.com>
Co-authored-by: Yusinto Ngadiman <yus@launchdarkly.com>
LaunchDarklyReleaseBot pushed a commit that referenced this issue Aug 15, 2023
Bumps [semver](https://github.com/npm/node-semver) from 5.7.1 to 5.7.2.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/npm/node-semver/releases">semver's
releases</a>.</em></p>
<blockquote>
<h2>v5.7.2</h2>
<h2><a
href="https://github.com/npm/node-semver/compare/v5.7.1...v5.7.2">5.7.2</a>
(2023-07-10)</h2>
<h3>Bug Fixes</h3>
<ul>
<li><a
href="https://github.com/npm/node-semver/commit/2f8fd41487acf380194579ecb6f8b1bbfe116be0"><code>2f8fd41</code></a>
<a href="https://redirect.github.com/npm/node-semver/pull/585">#585</a>
better handling of whitespace (<a
href="https://redirect.github.com/npm/node-semver/issues/585">#585</a>)
(<a href="https://github.com/joaomoreno"><code>@​joaomoreno</code></a>,
<a
href="https://github.com/lukekarrys"><code>@​lukekarrys</code></a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/npm/node-semver/blob/v5.7.2/CHANGELOG.md">semver's
changelog</a>.</em></p>
<blockquote>
<h2><a
href="https://github.com/npm/node-semver/compare/v5.7.1...v5.7.2">5.7.2</a>
(2023-07-10)</h2>
<h3>Bug Fixes</h3>
<ul>
<li><a
href="https://github.com/npm/node-semver/commit/2f8fd41487acf380194579ecb6f8b1bbfe116be0"><code>2f8fd41</code></a>
<a href="https://redirect.github.com/npm/node-semver/pull/585">#585</a>
better handling of whitespace (<a
href="https://redirect.github.com/npm/node-semver/issues/585">#585</a>)
(<a href="https://github.com/joaomoreno"><code>@​joaomoreno</code></a>,
<a
href="https://github.com/lukekarrys"><code>@​lukekarrys</code></a>)</li>
</ul>
<h2>5.7</h2>
<ul>
<li>Add <code>minVersion</code> method</li>
</ul>
<h2>5.6</h2>
<ul>
<li>Move boolean <code>loose</code> param to an options object, with
backwards-compatibility protection.</li>
<li>Add ability to opt out of special prerelease version handling with
the <code>includePrerelease</code> option flag.</li>
</ul>
<h2>5.5</h2>
<ul>
<li>Add version coercion capabilities</li>
</ul>
<h2>5.4</h2>
<ul>
<li>Add intersection checking</li>
</ul>
<h2>5.3</h2>
<ul>
<li>Add <code>minSatisfying</code> method</li>
</ul>
<h2>5.2</h2>
<ul>
<li>Add <code>prerelease(v)</code> that returns prerelease
components</li>
</ul>
<h2>5.1</h2>
<ul>
<li>Add Backus-Naur for ranges</li>
<li>Remove excessively cute inspection methods</li>
</ul>
<h2>5.0</h2>
<ul>
<li>Remove AMD/Browserified build artifacts</li>
<li>Fix ltr and gtr when using the <code>*</code> range</li>
<li>Fix for range <code>*</code> with a prerelease identifier</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/npm/node-semver/commit/f8cc313550691a50d9662d8c94f0c033717efd7d"><code>f8cc313</code></a>
chore: release 5.7.2</li>
<li><a
href="https://github.com/npm/node-semver/commit/2f8fd41487acf380194579ecb6f8b1bbfe116be0"><code>2f8fd41</code></a>
fix: better handling of whitespace (<a
href="https://redirect.github.com/npm/node-semver/issues/585">#585</a>)</li>
<li><a
href="https://github.com/npm/node-semver/commit/deb5ad51bf58868fa243c1683775305fe9e0e365"><code>deb5ad5</code></a>
chore: <code>@​npmcli/template-oss</code><a
href="https://github.com/4"><code>@​4</code></a>.16.0</li>
<li>See full diff in <a
href="https://github.com/npm/node-semver/compare/v5.7.1...v5.7.2">compare
view</a></li>
</ul>
</details>
<details>
<summary>Maintainer changes</summary>
<p>This version was pushed to npm by <a
href="https://www.npmjs.com/~lukekarrys">lukekarrys</a>, a new releaser
for semver since your current version.</p>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=semver&package-manager=npm_and_yarn&previous-version=5.7.1&new-version=5.7.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/launchdarkly/react-native-client-sdk-private/network/alerts).

</details>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants