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

Need ability to cancel a message push #45

Closed
mrj opened this issue Sep 6, 2015 · 4 comments
Closed

Need ability to cancel a message push #45

mrj opened this issue Sep 6, 2015 · 4 comments

Comments

@mrj
Copy link

mrj commented Sep 6, 2015

When a pushMessage (formerly send) Promise is unresolved, do subsequent calls queue, replace, or are they rejected?

In any case, there needs to be a mechanism for cancelling a message push.

Also, what is the meaning of

  1. If after making the request the connection is lost, reject promise with "TimeoutError"

in the draft API? Is this when data transfer is incomplete?

@zolkis
Copy link
Contributor

zolkis commented Sep 6, 2015

Setting a push message, either to a tag or to a peer (works the same way through this interface) means that the user intends to send data, and needs to physically touch the target. If that does not happen within a timeout, or if there is a connection but it's lost while communicating, then there is an error, and promise is rejected with a timeout.

If a subsequent call of pushMessage happens, then the message is updated and the timer reset. There is no "cancellation", since a physical connection/communication did not yet happen.
Calling pushMessage during communication is probably a bug in the client code, though theoretically possible, and then we have a race condition indeed - but that is completely up to the client if they want to use it that way, and the existence of a cancel method won't help that (they might choose to not use it). Implementations are expected to treat the communication time a critical section, so that the update of the push message never interleaves with the transmission itself. Basically this is already ensured by the nature of native API's, so I don't see a real threat of a race condition there.

IMO we don't need a separate cancel method, just to explain the above in the steps. If that is fine, I will make a new commit in this PR to fix it. Thanks for the observation.

@mrj
Copy link
Author

mrj commented Sep 6, 2015

If new messages update pending ones, then given the max. 10s timeout, there's less need for a cancel. There is though a bit of a security worry that there's up to a 10s window when data could be sent to an unauthorised party if the user is distracted after initiating a send, with no way to cancel.

What happens to a pending message if the sending page is closed or becomes no longer visible?

I agree that the semantics of pushing an updated message during the data transfer period is less important. Queuing or rejection may be the best choice, because a delayed pushMessage return would be unexpected for an asynchronous command, and may lock-up the UI.

@zolkis
Copy link
Contributor

zolkis commented Sep 7, 2015

The assumption/use case is that the user wants to send data through NFC, press a button, the page sets up the (selected) data for the push message, starts the timer, and the user has a given time window to do the touch gesture, depending on what the page sets. It is unlikely the user gets distracted, since Web NFC only works when the page is in focus, the display is on, etc. If the user switches apps to do something else during the time window of the expected gesture, NFC functionality is stopped and the timer will likely expire. If the user just daydreams for a few seconds, forgets what s/he wanted, then touch the phone to another tag or device while the same page is open and in focus, then it was her/his decision and we should not try to fix that from the API :).

However, if the page wants to show a button for canceling the push, then we may need a cancelPush() for clarity's sake, though in practice it's a race too, difficult to guarantee the "cancel" semantics and in fact no better than its ugly equivalent of
adapter.pushMessage(null, { timeout: 0});
So we may have that as a syntactic sugar.

@alexshalamov
Copy link

I think we can have proper "cancel" method in next version of the spec. It is implementable on all platforms. It would be cleaner than pushing null message with 0 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

No branches or pull requests

3 participants