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

Add a specific exception for HandshakeTimeout ? #1223

Closed
sbernard31 opened this issue Feb 18, 2020 · 8 comments · Fixed by #1226
Closed

Add a specific exception for HandshakeTimeout ? #1223

sbernard31 opened this issue Feb 18, 2020 · 8 comments · Fixed by #1226
Milestone

Comments

@sbernard31
Copy link
Contributor

In Leshan, when a request timeout for any reason we raise a TimeoutException.
We decide recently to consider a Handshake flight timeout as Leshan request timeout.

But currently there no easy way to know handshake timeout because we raise an Exception.
So the code looks like something like this :

    if (error instanceof Exception && error.getMessage().startsWith("Handshake flight")) {

Should we raise a specific exception ?

(Ideally I would like to be able to distinguish a CoAP timeout from a Blockwise timeout but I guess this would be a really more complex task , this is maybe relative to #818 ?)

@boaks
Copy link
Contributor

boaks commented Feb 18, 2020

Ideally I would like to be able to distinguish a CoAP timeout from a Blockwise timeout but I guess this would be a really more complex task

is the "CoAP timeout" the timeout of a single request? And the blockwise timeout then the timeout of the complete blockwise transfer?

@sbernard31
Copy link
Contributor Author

sbernard31 commented Feb 19, 2020

Nope, I try to explain "my terminology" in this wiki page. Let me know if this is not clear.

My idea is to let user aware of the cause/type of the timeout and so he will be able to change it if needed.

@boaks
Copy link
Contributor

boaks commented Feb 19, 2020

  1. a specific exception for handshake timeouts should be possible.

  2. the CoAP timeout should already work (Message.isTimedOut()). Do you see any issue with it?

  3. the blockwise timeout seems to be more your domain (at least it's based on your contributions). My impression is, that this timeout mainly occurs on the server-side to free the resource payload.

@sbernard31
Copy link
Contributor Author

  1. A specific exception for handshake timeouts should be possible.

👍 !

  1. The CoAP timeout should already work (Message.isTimedOut()). Do you see any issue with it?

AFAIK, there is no issue here. (In leshan we use MessageObserver.onTimeout(). (see eclipse-leshan/leshan@3e10026)

  1. the blockwise timeout seems to be more your domain (at least it's based on your contributions). My impression is, that this timeout mainly occurs on the server-side to free the resource payload.

I think you're right in normal use case this should mainly be a timeout at server side.
The only exception I see is if CON is not used or piggybacked is not used (which is maybe out of spec) in the case request could be timed-out (so at client side). Anyway as I said I can not see easy way to do that without modifying the API. (This is more a midterm/longterm thinking)

@boaks
Copy link
Contributor

boaks commented Feb 19, 2020

That depends then on the implementation of that timeout.

Response response = send(request, outEndpoint).waitForResponse(timeout);
if (response == null) {
	// Cancel request so appropriate clean up can happen.
	request.cancel();

something like that would trigger:

private MessageObserver addBlock2CleanUpObserver(final Request message, final KeyUri key, final Block2BlockwiseStatus status) {

	MessageObserver observer = new MessageObserverAdapter() {

		@Override
		public void onCancel() {
			clearBlock2Status(key, status);
		}

		@Override
		protected void failed() {
			clearBlock2Status(key, status);
		}
	};
	message.addMessageObserver(observer);
	return observer;
}

and so even that may not end up in a blockwise timeout.

@sbernard31
Copy link
Contributor Author

🤔 in that case I feel it normal as we cancel the request before, or I missed something ?

@boaks
Copy link
Contributor

boaks commented Feb 19, 2020

in that case I feel it normal as we cancel the request before

What is considered to be normal? The blockwise timeout?

@sbernard31
Copy link
Contributor Author

I was talking about :

so even that may not end up in a blockwise timeout.

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 a pull request may close this issue.

2 participants