urbit/shrub
1
1
Fork 0
mirror of https://github.com/urbit/shrub.git synced 2024-11-23 20:26:54 +03:00
Code Issues Packages Projects Releases Wiki Activity

eyre: GETting non-existent channels 404s

Browse Source 
Contrary to the argument made by 4affae8, this is the _actually correct_
behavior. Not creating server-side resources in response to GETs
respects the expected method semantics, and more importantly, serving a
404 is an important signal for clients trying to connect to a channel
they were using previously. Without that, they have no way of telling
whether, when reconnecting, if their channel was reaped in the mean time
or not.

The "empty PUT" affordance provided by 34148f9 makes requiring a PUT
request for channel creation more reasonable.

We leave the general refactoring done by #6789 in place, but do
emphasize the reasoning given here with a few additional comments.
This commit is contained in:
fang 2023-10-04 22:42:50 +02:00
parent bdb906340d
commit 14266c8d46
No known key found for this signature in database
GPG Key ID: EB035760C1BBA972
2 changed files with 31 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
pkg/arvo/sys/vane/eyre.hoon
View File

@ -2191,12 +2191,20 @@
[%b %rest expiration-time]
:: +on-get-request: handles a GET request
::
:: GET requests open a channel for the server to send events to the
:: client in text/event-stream format.
:: GET requests connect to a channel for the server to send events to
:: the client in text/event-stream format.
::
++ on-get-request
|= [channel-id=@t [session-id=@uv =identity] =request:http]
^- [(list move) server-state]
:: if the channel doesn't exist, we cannot serve it.
:: this 404 also lets clients know if their channel was reaped since
:: they last connected to it.
::
?. (~(has by session.channel-state.state) channel-id)
%^ return-static-data-on-duct 404 'text/html'
(error-page 404 | url.request ~)
::
=/ mode=?(%json %jam)
(find-channel-mode %'GET' header-list.request)
=^ [exit=? =wall moves=(list move)] state
@ -2206,23 +2214,14 @@
?~ maybe-raw-header=(get-header:http 'last-event-id' header-list.request)
~
(rush u.maybe-raw-header dum:ag)
:: if the channel doesn't exist yet, simply instantiate it here
::
?~ maybe-channel=(~(get by session.channel-state.state) channel-id)
=- [[| ~ ~] state(session.channel-state -)]
%+ ~(put by session.channel-state.state) channel-id
::NOTE some other fields initialized at the end of this arm
%* . *channel
identity identity
next-id (fall maybe-last-event-id 0)
last-ack now
==
:: if the channel does exist, we put some demands on the get request,
:: and may need to do some cleanup for prior requests.
=/ channel
(~(got by session.channel-state.state) channel-id)
:: we put some demands on the get request, and may need to do some
:: cleanup for prior requests.
::
:: find the channel creator's identity, make sure it matches
::
?. =(identity identity.u.maybe-channel)
?. =(identity identity.channel)
=^ mos state
%^ return-static-data-on-duct 403 'text/html'
(error-page 403 | url.request ~)
@ -2231,34 +2230,34 @@
::
::TODO or could we change that on the spot, given that only a single
:: request will ever be listening to this channel?
?. =(mode mode.u.maybe-channel)
?. =(mode mode.channel)
=^ mos state
%^ return-static-data-on-duct 406 'text/html'
=; msg=tape (error-page 406 %.y url.request msg)
"channel already established in {(trip mode.u.maybe-channel)} mode"
"channel already established in {(trip mode.channel)} mode"
[[& ~ mos] state]
:: when opening an event-stream, we must cancel our timeout timer
:: if there's no duct already bound. else, kill the old request,
:: we will replace its duct at the end of this arm
::
=^ cancel-moves state
?: ?=([%& *] state.u.maybe-channel)
?: ?=([%& *] state.channel)
:_ state
(cancel-timeout-move channel-id p.state.u.maybe-channel)^~
(cancel-timeout-move channel-id p.state.channel)^~
=. duct-to-key.channel-state.state
(~(del by duct-to-key.channel-state.state) p.state.u.maybe-channel)
(~(del by duct-to-key.channel-state.state) p.state.channel)
=/ cancel-heartbeat
?~ heartbeat.u.maybe-channel ~
?~ heartbeat.channel ~
:_ ~
%+ cancel-heartbeat-move channel-id
[date duct]:u.heartbeat.u.maybe-channel
[date duct]:u.heartbeat.channel
=- [(weld cancel-heartbeat -<) ->]
(handle-response(duct p.state.u.maybe-channel) [%cancel ~])
(handle-response(duct p.state.channel) [%cancel ~])
:: flush events older than the passed in 'Last-Event-ID'
::
=? state ?=(^ maybe-last-event-id)
(acknowledge-events channel-id u.maybe-last-event-id)
::TODO that did not remove them from the u.maybe-channel queue though!
::TODO that did not remove them from the channel queue though!
:: we may want to account for maybe-last-event-id, for efficiency.
:: (the client _should_ ignore events it heard previously if we do
:: end up re-sending them, but _requiring_ that feels kinda risky)
@ -2269,7 +2268,7 @@
[[| - cancel-moves] state]
%- zing
%- flop
=/ queue events.u.maybe-channel
=/ queue events.channel
=| events=(list wall)
|-
^+ events
@ -2281,7 +2280,7 @@
:: since conversion failure also gets caught during first receive.
:: we can't do anything about this, so consider it unsupported.
=/ said
(channel-event-to-tape u.maybe-channel request-id channel-event)
(channel-event-to-tape channel request-id channel-event)
?~ said $
$(events [(event-tape-to-wall id +.u.said) events])
?: exit [moves state]
@ -2346,6 +2345,8 @@
::
:: PUT requests send commands from the client to the server. We receive
:: a set of commands in JSON format in the body of the message.
:: channels don't exist until a PUT request is sent. it's valid for
:: this request to contain an empty list of commands.
::
++ on-put-request
|= [channel-id=@t =identity =request:http]

7
tests/sys/vane/eyre.hoon
View File

@ -749,13 +749,12 @@
;< ~ bind:m perform-init-wo-timer
;< ~ bind:m perform-born
;< ~ bind:m (wait ~d1)
;< ~ bind:m perform-authentication-2
;< mos=(list move) bind:m
(get '/~/channel/0123456789abcdef' cookie)
;< now=@da bind:m get-now
=/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' (add now ~s20))
=/ mov-2 (ex-channel-response ~)
(expect-moves mos mov-1 mov-2 ~)
=/ headers ['content-type' 'text/html']~
=/ body `(error-page:eyre-gate 404 %.n '/~/channel/0123456789abcdef' ~)
(expect-moves mos (ex-response 404 headers body) ~)
::
++ test-channel-put-zero-requests
%- eval-mare