Add callback mechanism into LwM2mClientObserver for when the unexpected error occurred #941

moznion · 2020-11-30T17:02:14Z

Done

Add onUnexpectedErrorOccurred() method to LwM2mClientObserver interface
Call LwM2mClientObserver#onUnexpectedErrorOccurred() on DefaultRegistrationEngine when it raises RuntimeException at task loop.
Support this error handling system on the client demo project

…2mClientObserver Add an interface method `void onUnexpectedErrorOccurred(Throwable unexpectedError)` into LwM2mClientObserver. This aims to hook a procedure when an unexpected error has been occurred. Signed-off-by: moznion <[email protected]>

…ngine` Call `LwM2mClientObserver#onUnexpectedErrorOccurred()` on `DefaultRegistrationEngine` when it raises `RuntimeException` at task loop. Signed-off-by: moznion <[email protected]>

…r has occurred Signed-off-by: moznion <[email protected]>

sbernard31 · 2020-12-01T14:27:02Z

leshan-client-demo/src/main/java/org/eclipse/leshan/client/demo/LeshanClientDemo.java

@@ -745,6 +748,9 @@ public void handshakeFailed(Handshaker handshaker, Throwable error) {
        builder.setBootstrapAdditionalAttributes(bsAdditionalAttributes);
        final LeshanClient client = builder.build();

+        client.addObserver(
+            new ClientShutdownOnUnexpectedErrorObserver(client, myDevice, randomTemperatureSensor));


This should be done on LeshanClient not LeshanClientDemo or I missed something ?
(#933 (comment))

Yes, that's a point. I think it would be better to implement this shutdown observer mechanism on LeshanClient, but it breaks the backward compatibility of the client, so I delegated this behavior to the client of the client.

What do you think about this?

For master, we don't care about backward compatibility question and should just focus on what could be the better behavior. (of course this doesn't mean we can break everything without any reason 😅 )

For 1.x, we should try to not break the API and behavior but here we could almost consider this as a bug ... so changing this behavior doesn't disturb me.

WDYT ?

Yeah, I'm on the same page.
I also think it would be better to implement this on the LeshanClient.
I was scared to break the backward compatibility way too much, but it seems no problem. I'll do this.

One more thing. Would it useful to support a method to prevent shutting down even if an unexpected error has occurred on the LeshanClient? For example LeshanClient#doNotDestroyOnUnexpectedError().

I would not add it for now. Let's see if someone need it later.

I still don't see what could be the use case. As tasks in DefaultRegistrationEngine are stopped. 🤔

yeah, my suggestion was for just in case. It looks there is no reason to support that. Thank you.

fixed at 7e6d029

sbernard31 · 2020-12-01T14:27:27Z

leshan-client-demo/src/main/java/org/eclipse/leshan/client/demo/LeshanClientDemo.java

+            myDevice.destroy();
+            randomTemperatureSensor.destroy();


I think we need to do the same Lwm2mInstanceEnabler should also implement (startable, stopable, destroyable)
And Object enabler should do pretty much the same as "LwM2mObjectTree" by interating over instance. (see #933 (comment))

Sounds good. I'll do this 👍

fixed at 3f73020

sbernard31 · 2020-12-01T14:29:48Z

leshan-client-core/src/main/java/org/eclipse/leshan/client/observer/LwM2mClientObserver.java

+
+    // ============== Unexpected Error Handling =================
+
+    void onUnexpectedErrorOccurred(Throwable unexpectedError);


maybe onUnexpectedError is enough ?

Thank you for the advice, I've not had confidence in this honestly :)

fixed at 16c6cb7

…rror/g Signed-off-by: moznion <[email protected]>

… ObjectEnabler To call `LwM2mInstanceEnabler#stop()`, `LwM2mInstanceEnabler#start()` and `LwM2mInstanceEnabler#destroy()` on the associated method, for each enabler if implemented the interface. Signed-off-by: moznion <[email protected]>

…shanClient Instead of on `LeshanClientDemo` Signed-off-by: moznion <[email protected]>

Signed-off-by: moznion <[email protected]>

sbernard31 · 2020-12-07T13:37:35Z

I integrated this in master (commits d0f2705, a1bacb5, 1da2600, 7a013f0)

I rewrite history to get more consistent commits.
I fixed a javadoc formatting issue in LwM2mInstanceEnabler.
I change a little bit the last commit by using anonymous class (see 7a013f0), in this commit I also changes LeshanClient.this.destroy(true); to LeshanClient.this.destroy(false); (because I was afraid in some condition that we could face a kind of infinite loop)

Thx @moznion for this new contribution 🙏 !

moznion · 2020-12-08T02:10:12Z

Many thanks for your support, @sbernard31 !

moznion added 3 commits December 1, 2020 01:57

eclipse-leshan#933: Handle unexpected errors on `DefaultRegistrationE…

7e32bf5

…ngine` Call `LwM2mClientObserver#onUnexpectedErrorOccurred()` on `DefaultRegistrationEngine` when it raises `RuntimeException` at task loop. Signed-off-by: moznion <[email protected]>

eclipse-leshan#933: Add demo code to shutdown when an unexpected erro…

e3aad0d

…r has occurred Signed-off-by: moznion <[email protected]>

sbernard31 reviewed Dec 1, 2020

View reviewed changes

moznion added 3 commits December 4, 2020 00:38

eclipse-leshan#933: Rename: s/onUnexpectedErrorOccurred/onUnexpectedE…

16c6cb7

…rror/g Signed-off-by: moznion <[email protected]>

eclipse-leshan#933: register ShutdownOnUnexpectedErrorObserver for Le…

7e6d029

…shanClient Instead of on `LeshanClientDemo` Signed-off-by: moznion <[email protected]>

moznion requested a review from sbernard31 December 3, 2020 15:41

eclipse-leshan#933: format code

b4a89c9

Signed-off-by: moznion <[email protected]>

sbernard31 closed this Dec 7, 2020

moznion deleted the 933_unexpected_error_callback branch December 8, 2020 02:09

moznion mentioned this pull request Dec 8, 2020

Backport: error handling for unexpected error in DefaultRegistrationEngine #944

Closed

sbernard31 mentioned this pull request Dec 10, 2020

Better handle unexpected error in DefaultRegistrationEngine. #933

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add callback mechanism into LwM2mClientObserver for when the unexpected error occurred #941

Add callback mechanism into LwM2mClientObserver for when the unexpected error occurred #941

moznion commented Nov 30, 2020

sbernard31 Dec 1, 2020

moznion Dec 2, 2020

sbernard31 Dec 2, 2020

moznion Dec 2, 2020

sbernard31 Dec 2, 2020

sbernard31 Dec 2, 2020 •

edited

Loading

moznion Dec 2, 2020

moznion Dec 3, 2020

sbernard31 Dec 1, 2020

moznion Dec 2, 2020

moznion Dec 3, 2020

sbernard31 Dec 1, 2020

moznion Dec 2, 2020

moznion Dec 3, 2020

sbernard31 commented Dec 7, 2020 •

edited

Loading

moznion commented Dec 8, 2020


		// ============== Unexpected Error Handling =================

		void onUnexpectedErrorOccurred(Throwable unexpectedError);

Add callback mechanism into LwM2mClientObserver for when the unexpected error occurred #941

Add callback mechanism into LwM2mClientObserver for when the unexpected error occurred #941

Conversation

moznion commented Nov 30, 2020

Done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbernard31 Dec 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbernard31 commented Dec 7, 2020 • edited Loading

moznion commented Dec 8, 2020

sbernard31 Dec 2, 2020 •

edited

Loading

sbernard31 commented Dec 7, 2020 •

edited

Loading