Editorial: correct command return values

[jugglinmike]

• Editorial: correct command return value

  The "handle an incoming message" algorithm expects every command to return either a "success" value or an "error" value.

   1. Let |result| be the result of running the [=remote end steps=] for
      |command| given |session| and [=command parameters=]
      |matched|["<code>params</code>"]
  
  1. If |result| is an [=error=], then [=send an error response=] given
     |connection|, |command id|, and |result|'s [=error code=], and finally
     return.
  
  1. Let |value| be |result|'s data.
  

  Update the command algorithms to satisfy this expectation.

• Editorial: return appropriate val. for EmptyResult

  The "handle an incoming message" algorithm asserts that values returned by command algorithms always match the documented type:

   1. Let |result| be the result of running the [=remote end steps=] for
      |command| given |session| and [=command parameters=]
      |matched|["<code>params</code>"]
  
  1. If |result| is an [=error=], then [=send an error response=] given
     |connection|, |command id|, and |result|'s [=error code=], and finally
     return.
  
  1. Let |value| be |result|'s data.
  
  1. Assert: |value| matches the definition for the [=result type=]
     corresponding to the command with [=command name=] |method|.
  

  However, some algorithms which are documented to return the EmptyResult type instead return null. Update those algorithms to return an empty map in order to satisfy the assertion.

---

Preview | Diff

[jgraham]

I thought I'd already fixed this by allowing the top-level algorithm to handle null, but apparently not. In any case I prefer that fix to this one just because it's rather verbose to always have to return an empty map.

[jgraham]

#351 is the alternative here.

[jugglinmike]

I thought I'd already fixed this by allowing the top-level algorithm to handle null, but apparently not. In any case I prefer that fix to this one just because it's rather verbose to always have to return an empty map.

That makes the relevant commands just a little harder to interpret, since readers will have to learn that null is a special value which gets translated into something else.

There may be a middle ground, though--one that isn't quite as wordy as what I've suggested so far and which also preserves the direct relationship between the return value and the data sent to the local end. What about:

-1. Return [=success=] with data null.
+1. Return [=success=] with data set to an empty map.

[jgraham]

I think I thought I'd fixed this because we fixed it in the original WebDriver spec.

I don't really mind either way between with data set to an empty map and with data null plus a special "null means an empty map" handling. The fact that no one really noticed for multiple years that the spec was broken, but nevertheless managed to come up with interoperable implementations suggests that the supposed difficulty reading it isn't much of a problem in practice.

In either case #351 now has a fix for the description of remote end steps incorrectly claiming that they return something matching the result production directly when in fact it's always a success or error value.