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

Gracefully handle null data in refresh offer message and log error #5788

Merged
merged 2 commits into from Nov 4, 2021
Merged

Gracefully handle null data in refresh offer message and log error #5788

merged 2 commits into from Nov 4, 2021

Conversation

ghost
Copy link

@ghost ghost commented Oct 29, 2021

User reported an unhandled exception popup in v1.7.5.

Just before the issue the client did get a disconnect: b.n.p.n.Connection: proto is null because protoInputStream.read()=-1 (EOF). That is expected if client got stopped without proper shutdown.

A offer refresh TTL message should not contain incomplete data, but is possible if the network connection is interrupted. There are other examples of interrupted network messages already logged.

Fix is to catch and log the exception stack trace. Just adds a try..catch block in P2PDataStorage.refreshTTL.
Fixes #5786

If there is an alternative way to handle this, let me know. @bisq-network/bisq-devs


Before

crash_p2p_exception

After:

[[NO POPUP SHOWN]]

and logfile contains a stack trace of the error:

WARN  b.n.p.n.Connection: proto is null because protoInputStream.read()=-1 (EOF). That is expected if client got stopped without proper shutdown. 
java.lang.IllegalArgumentException: Cannot create P2PDataStorage.ByteArray with null byte[] array argument.
    at bisq.network.p2p.storage.P2PDataStorage$ByteArray.verifyBytesNotEmpty(P2PDataStorage.java:1193)
    at bisq.network.p2p.storage.P2PDataStorage$ByteArray.<init>(P2PDataStorage.java:1188)
    at bisq.network.p2p.storage.P2PDataStorage.refreshTTL(P2PDataStorage.java:850)
    at bisq.network.p2p.storage.P2PDataStorage.lambda$onMessage$22(P2PDataStorage.java:585)
    at java.base/java.util.Optional.ifPresent(Optional.java:183)
    at bisq.network.p2p.storage.P2PDataStorage.onMessage(P2PDataStorage.java:577)
    at bisq.network.p2p.network.NetworkNode.lambda$onMessage$6(NetworkNode.java:392)
    at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
    at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:658)
    at bisq.network.p2p.network.NetworkNode.onMessage(NetworkNode.java:392)
    at bisq.network.p2p.network.Connection.lambda$onBundleOfEnvelopes$11(Connection.java:470)
    at java.base/java.util.concurrent.CopyOnWriteArrayList.forEach(CopyOnWriteArrayList.java:803)
    at java.base/java.util.concurrent.CopyOnWriteArraySet.forEach(CopyOnWriteArraySet.java:425)
    at bisq.network.p2p.network.Connection.lambda$onBundleOfEnvelopes$12(Connection.java:470)
    at com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:428)
    at java.base/java.security.AccessController.doPrivileged(Native Method)
    at com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:427)
    at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96)
    at com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
    at com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$11(GtkApplication.java:277)
    at java.base/java.lang.Thread.run(Thread.java:834)

@ghost
Copy link
Author

ghost commented Nov 2, 2021

Per request, added error log message. It looks like this:

Nov-02 09:28:57.426 [JavaFX Application Thread] ERROR b.n.p2p.storage.P2PDataStorage: refreshTTL failed, missing data: java.lang.IllegalArgumentException: Cannot create P2PDataStorage.ByteArray with null byte[] array argument.
[+followed by stack trace]

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link
Contributor

@chimp1984 chimp1984 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@ripcurlx ripcurlx merged commit 2d3cf38 into bisq-network:master Nov 4, 2021
@ripcurlx ripcurlx added this to the v1.8.0 milestone Nov 4, 2021
@ghost ghost mentioned this pull request Nov 18, 2021
@ghost ghost deleted the fix_refresh_ttl_exception branch May 29, 2022 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot create P2PDataStorage.ByteArray with empty byte[] array argument.
2 participants