Backport: error handling for unexpected error in DefaultRegistrationEngine #944
Conversation
…e and Stoppable Signed-off-by: moznion <[email protected]>
…able, Stoppable) And make a suggestion to use interfaces that are in the `core` package. Signed-off-by: moznion <[email protected]>
…r ObjectEnabler To call each interface method on corresponded `LwM2mObjectTree` method. Signed-off-by: moznion <[email protected]>
…r LwM2mObjectTree And call each interface's method at related method of `LeshanClient`; i.e. `start()`, `stop()` and `destroy()`. Signed-off-by: moznion <[email protected]>
… unexpected erorr This interface extends `LwM2mClientObserver` interface. And make `LwM2mClientObserverAdapter` and `LwM2mClientObserverDispatcher` implement that new interface. This doesn't break the backward compatibility because `LwM2mClientObserver2` is compatible with `LwM2mClientObserver` and each implementing class conceals the difference between `LwM2mClientObserver` and `LwM2mClientObserver2`. Signed-off-by: moznion <[email protected]>
…occurred If a task that are belong to `DefaultRegistrationEngine` raises unexpected `RuntimeException` and the `observer` member variable implements `LwM2mClientObserver2` (instead of `LwM2mClientObserver`), it calls `LwM2mClientObserver2#onUnexpectedError()` hook. The purpose of this hook gimmick is to shutdown the client application mainly. Signed-off-by: moznion <[email protected]>
…client And implements `Destroyable` interface for each threading task that runs with the demo client application. The reason why it doesn't set the hook at `LeshanClient` is to keep the backward compatibility, this means it delegates "what should client do once an unexpected error has occurred" to the users. Signed-off-by: moznion <[email protected]>
Signed-off-by: moznion <[email protected]>
To clarify the version that is going to remove the deprecated interfaces. Signed-off-by: moznion <[email protected]>
There was a problem hiding this comment.
This is good.
I just detect some issue about backward behavior compatibility about Destroyable, Startable, Stoppable interface and so create a dedicated PR #945. This way you can look at the modif before I integrate it.
I also squashed some of your commits to make it more consistent.
The remaining question should we shutdown on LeshanClient or LeshanClientDemo ?
| /** | ||
| * @deprecated please consider to use {@link org.eclipse.leshan.core.Destroyable} instead. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
Everywhere we use the old Destroyable interface we should now be able to use the new one.
(I created a new PR with this modification : df5725c)
| @@ -807,4 +809,18 @@ public void run() { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| private static class ShutdownOnUnexpectedErrorObserver extends LwM2mClientObserverAdapter { | |||
There was a problem hiding this comment.
I hope I'm not too boring with this but are you sure we should keep the backward compatibility behavior ?
As I said in #941 (comment), we could maybe consider the current behavior as a bug rather as a behavior which should not be changed ?
WDYT ?
Of course if you insist we can only add this behavior to LeshanClientDemo but it's hard to me to find why keeping a kind of process zombie alive should be a good idea.(#933 (comment))
There was a problem hiding this comment.
Yeah, this has been a point for consideration.
The previous pull-request (#941) was for the new major version 2.0, on the other hand, this pull-request aims to update the current stable version. So I'm a little bit afraid of breaking the backward compatibility...
But, as you mentioned, it is difficult to find out the reason to encourage keeping the living-dead client process.
This is just my opinion, it would be better to set the hook in LeshanClinet; but anyway I'd like to follow and respect the decision of this project. Let's do the same thing with #941?
There was a problem hiding this comment.
Ok so I cherrypick the commit from master instead of the one in this PR.
|
I close this one which is contained in #945. |
This pull-request backports the changes of #940 and #941 to
1.xversion.See also: #933.