Skip to content

Commit 10eb799

Browse files
committed
Detect final element
Closes #114.
1 parent 187fb46 commit 10eb799

File tree

10 files changed

+170
-84
lines changed

10 files changed

+170
-84
lines changed

.github/workflows/haskell-ci.yml

+6-22
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
#
99
# For more information, see https://github.com/haskell-CI/haskell-ci
1010
#
11-
# version: 0.19.20240501
11+
# version: 0.19.20240514
1212
#
13-
# REGENDATA ("0.19.20240501",["github","cabal.project.ci"])
13+
# REGENDATA ("0.19.20240514",["github","cabal.project.ci"])
1414
#
1515
name: Haskell-CI
1616
on:
@@ -28,9 +28,9 @@ jobs:
2828
strategy:
2929
matrix:
3030
include:
31-
- compiler: ghc-9.10.0.20240426
31+
- compiler: ghc-9.10.1
3232
compilerKind: ghc
33-
compilerVersion: 9.10.0.20240426
33+
compilerVersion: 9.10.1
3434
setup-method: ghcup
3535
allow-failure: false
3636
- compiler: ghc-9.8.2
@@ -67,7 +67,6 @@ jobs:
6767
mkdir -p "$HOME/.ghcup/bin"
6868
curl -sL https://downloads.haskell.org/ghcup/0.1.20.0/x86_64-linux-ghcup-0.1.20.0 > "$HOME/.ghcup/bin/ghcup"
6969
chmod a+x "$HOME/.ghcup/bin/ghcup"
70-
"$HOME/.ghcup/bin/ghcup" config add-release-channel https://raw.githubusercontent.com/haskell/ghcup-metadata/master/ghcup-prereleases-0.0.8.yaml;
7170
"$HOME/.ghcup/bin/ghcup" install ghc "$HCVER" || (cat "$HOME"/.ghcup/logs/*.* && false)
7271
"$HOME/.ghcup/bin/ghcup" install cabal 3.10.2.0 || (cat "$HOME"/.ghcup/logs/*.* && false)
7372
apt-get update
@@ -94,7 +93,7 @@ jobs:
9493
echo "HCNUMVER=$HCNUMVER" >> "$GITHUB_ENV"
9594
echo "ARG_TESTS=--enable-tests" >> "$GITHUB_ENV"
9695
echo "ARG_BENCH=--enable-benchmarks" >> "$GITHUB_ENV"
97-
if [ $((HCNUMVER >= 91000)) -ne 0 ] ; then echo "HEADHACKAGE=true" >> "$GITHUB_ENV" ; else echo "HEADHACKAGE=false" >> "$GITHUB_ENV" ; fi
96+
echo "HEADHACKAGE=false" >> "$GITHUB_ENV"
9897
echo "ARG_COMPILER=--$HCKIND --with-compiler=$HC" >> "$GITHUB_ENV"
9998
echo "GHCJSARITH=0" >> "$GITHUB_ENV"
10099
env:
@@ -123,18 +122,6 @@ jobs:
123122
repository hackage.haskell.org
124123
url: http://hackage.haskell.org/
125124
EOF
126-
if $HEADHACKAGE; then
127-
cat >> $CABAL_CONFIG <<EOF
128-
repository head.hackage.ghc.haskell.org
129-
url: https://ghc.gitlab.haskell.org/head.hackage/
130-
secure: True
131-
root-keys: 7541f32a4ccca4f97aea3b22f5e593ba2c0267546016b992dfadcd2fe944e55d
132-
26021a13b401500c8eb2761ca95c61f2d625bfef951b939a8124ed12ecf07329
133-
f76d08be13e9a61a377a85e2fb63f4c5435d40f8feb3e12eb05905edb8cdea89
134-
key-threshold: 3
135-
active-repositories: hackage.haskell.org, head.hackage.ghc.haskell.org:override
136-
EOF
137-
fi
138125
cat >> $CABAL_CONFIG <<EOF
139126
program-default-options
140127
ghc-options: $GHCJOBS +RTS -M3G -RTS
@@ -191,10 +178,7 @@ jobs:
191178
flags: +build-demo +build-stress-test +snappy
192179
ghc-options: -Werror
193180
EOF
194-
if $HEADHACKAGE; then
195-
echo "allow-newer: $($HCPKG list --simple-output | sed -E 's/([a-zA-Z-]+)-[0-9.]+/*:\1,/g')" >> cabal.project
196-
fi
197-
$HCPKG list --simple-output --names-only | perl -ne 'for (split /\s+/) { print "constraints: $_ installed\n" unless /^(grapesy)$/; }' >> cabal.project.local
181+
$HCPKG list --simple-output --names-only | perl -ne 'for (split /\s+/) { print "constraints: any.$_ installed\n" unless /^(grapesy)$/; }' >> cabal.project.local
198182
cat cabal.project
199183
cat cabal.project.local
200184
- name: dump install plan

grapesy.cabal

+2-2
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ library
194194
, exceptions >= 0.10 && < 0.11
195195
, hashable >= 1.3 && < 1.5
196196
, http-types >= 0.12 && < 0.13
197-
, http2 >= 5.2.1 && < 5.3
197+
, http2 >= 5.2.2 && < 5.3
198198
, http2-tls >= 0.2.11 && < 0.3
199199
, lens >= 5.0 && < 5.4
200200
, mtl >= 2.2 && < 2.4
@@ -305,7 +305,7 @@ test-suite test-grapesy
305305
, containers >= 0.6 && < 0.8
306306
, exceptions >= 0.10 && < 0.11
307307
, http-types >= 0.12 && < 0.13
308-
, http2 >= 5.2.1 && < 5.3
308+
, http2 >= 5.2.2 && < 5.3
309309
, mtl >= 2.2 && < 2.4
310310
, network >= 3.1 && < 3.3
311311
, proto-lens-runtime >= 0.7 && < 0.8

src/Network/GRPC/Client.hs

-4
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,3 @@ import Network.GRPC.Util.TLS qualified as Util.TLS
187187
-- The specialized functions from "Network.GRPC.Client.StreamType" take care of
188188
-- this; if these functions are not applicable, users may wish to use
189189
-- 'recvFinalOutput'.
190-
--
191-
-- **KNOWN LIMITATION**: for _incoming_ messages @grapesy@ cannot currently
192-
-- make this distinction. For detailed discussion, see
193-
-- <https://github.com/well-typed/grapesy/issues/114>.

test-grapesy/Test/Driver/Dialogue/Execution.hs

+6-5
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,12 @@ clientLocal clock call = \(LocalSteps steps) ->
206206
expect (tick, action) (== ResponseInitialMetadata expectedMetadata) $
207207
receivedMetadata
208208
Send (FinalElem a b) -> do
209-
-- <https://github.com/well-typed/grapesy/issues/114>
209+
-- On the client side, when the server sends the final message, we
210+
-- will receive that final message in one HTTP data frame, and then
211+
-- the trailers in another. This means that when we get the message,
212+
-- we do not yet know if this is in fact the last.
213+
-- (This is different on the server side, because gRPC does not
214+
-- support trailers on the client side.)
210215
reactToServer tick $ Send (StreamElem a)
211216
reactToServer tick $ Send (NoMoreElems b)
212217
Send expectedElem -> do
@@ -401,10 +406,6 @@ serverLocal clock call = \(LocalSteps steps) -> do
401406
case action of
402407
Initiate _ ->
403408
error "serverLocal: unexpected ClientInitiateRequest"
404-
Send (FinalElem a b) -> do
405-
-- <https://github.com/well-typed/grapesy/issues/114>
406-
reactToClient tick $ Send (StreamElem a)
407-
reactToClient tick $ Send (NoMoreElems b)
408409
Send expectedElem -> do
409410
mInp <- liftIO $ try $ within timeoutReceive action $
410411
Server.Binary.recvInput call

test-grapesy/Test/Prop/IncrementalParsing.hs

+15-5
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,12 @@ runPure chunks =
6464
. flip runStateT chunks
6565
. unwrapPure
6666

67-
getChunk :: Pure Strict.ByteString
67+
getChunk :: Pure (Strict.ByteString, Bool)
6868
getChunk =
6969
state $ \case
70-
[] -> (BS.Strict.empty, [])
71-
c:cs -> (c, cs)
70+
[] -> ((BS.Strict.empty, True), [])
71+
[c] -> ((c, True), [])
72+
c:cs -> ((c, False), cs)
7273

7374
processAll ::
7475
[Strict.ByteString]
@@ -85,8 +86,8 @@ processAll chunks processOne p =
8586
-- throw errors), so we can reuse that also for any parse failures.
8687
aux :: Pure Lazy.ByteString
8788
aux =
88-
Parser.processAll getChunk processOne p
89-
>>= either throwError return
89+
Parser.processAll getChunk processOne processOne p
90+
>>= throwParseErrors
9091

9192
-- 'processAll' should run until all chunks are used
9293
verifyAllChunksConsumed ::
@@ -99,6 +100,15 @@ processAll chunks processOne p =
99100
| otherwise
100101
= Left "not all chunks consumed"
101102

103+
-- TODO: Verify 'ProcessedFinal'/'ProcessedWithoutFinal'
104+
throwParseErrors :: Parser.ProcessResult String () -> Pure Lazy.ByteString
105+
throwParseErrors (Parser.ProcessError err) =
106+
throwError err
107+
throwParseErrors (Parser.ProcessedWithFinal () bs) =
108+
return bs
109+
throwParseErrors (Parser.ProcessedWithoutFinal bs) =
110+
return bs
111+
102112
{-------------------------------------------------------------------------------
103113
Test input
104114

test-grapesy/Test/Sanity/EndOfStream.hs

+33-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ tests = testGroup "Test.Sanity.EndOfStream" [
2929
, testCase "recvTrailers" test_recvTrailers
3030
]
3131
, testGroup "server" [
32-
testCase "recvEndOfInput" test_recvEndOfInput
32+
testCase "recvInput" test_recvInput
33+
, testCase "recvEndOfInput" test_recvEndOfInput
3334
]
3435
]
3536

@@ -155,6 +156,37 @@ serverStreamingHandler = Server.streamingRpcHandler $
155156
Server tests
156157
-------------------------------------------------------------------------------}
157158

159+
-- | Test that the final element is marked as 'FinalElem'
160+
--
161+
-- Verifies that <https://github.com/well-typed/grapesy/issues/114> is solved.
162+
--
163+
-- NOTE: There is no client equivalent for this test. On the client side, the
164+
-- server will send trailers, and so /cannot/ make the final data frame as
165+
-- end-of-stream.
166+
test_recvInput :: Assertion
167+
test_recvInput = testClientServer $ ClientServerTest {
168+
config = def
169+
, server = [Server.someRpcHandler handler]
170+
, client = simpleTestClient $ \conn ->
171+
Client.withRPC conn def (Proxy @Trivial) $ \call -> do
172+
Client.sendFinalInput call BS.Lazy.empty
173+
_resp <- Client.recvFinalOutput call
174+
return ()
175+
}
176+
where
177+
handler :: Server.RpcHandler IO Trivial
178+
handler = Server.mkRpcHandler $ \call -> do
179+
x <- Server.recvInput call
180+
181+
-- The purpose of this test:
182+
case x of
183+
FinalElem{} ->
184+
return ()
185+
_otherwise ->
186+
assertFailure "Expected FinalElem"
187+
188+
Server.sendFinalOutput call (mempty, NoMetadata)
189+
158190
-- | Test that 'recvEndOfInput' does /not/ throw an exception, even if the
159191
-- previous 'recvNextInput' /happened/ to give us the final input.
160192
--

test-grapesy/Test/Sanity/StreamingType/CustomFormat.hs

+2-1
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,11 @@ test_calculator_cbor = do
232232
biDiStreamingSumHandler = Server.streamingRpcHandler $
233233
Server.mkBiDiStreaming $ \recv send ->
234234
let
235+
go :: Int -> IO ()
235236
go acc =
236237
recv >>= \case
237238
NoMoreElems _ -> return ()
238-
FinalElem n _ -> send (acc + n) >> go (acc + n)
239+
FinalElem n _ -> send (acc + n)
239240
StreamElem n -> send (acc + n) >> go (acc + n)
240241
in
241242
go 0

util/Network/GRPC/Util/HTTP2/Stream.hs

+4-4
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ data OutputStream = OutputStream {
4040
}
4141

4242
data InputStream = InputStream {
43-
_getChunk :: HasCallStack => IO Strict.ByteString
43+
_getChunk :: HasCallStack => IO (Strict.ByteString, Bool)
4444
, _getTrailers :: HasCallStack => IO [HTTP.Header]
4545
}
4646

@@ -54,7 +54,7 @@ writeChunk = _writeChunk
5454
flush :: HasCallStack => OutputStream -> IO ()
5555
flush = _flush
5656

57-
getChunk :: HasCallStack => InputStream -> IO Strict.ByteString
57+
getChunk :: HasCallStack => InputStream -> IO (Strict.ByteString, Bool)
5858
getChunk = _getChunk
5959

6060
getTrailers :: HasCallStack => InputStream -> IO [HTTP.Header]
@@ -68,7 +68,7 @@ serverInputStream :: Server.Request -> IO InputStream
6868
serverInputStream req = do
6969
return InputStream {
7070
_getChunk = wrapStreamExceptionsWith ClientDisconnected $
71-
Server.getRequestBodyChunk req
71+
Server.getRequestBodyChunk' req
7272
, _getTrailers = wrapStreamExceptionsWith ClientDisconnected $
7373
maybe [] fromHeaderTable <$>
7474
Server.getRequestTrailers req
@@ -132,7 +132,7 @@ clientInputStream :: Client.Response -> IO InputStream
132132
clientInputStream resp = do
133133
return InputStream {
134134
_getChunk = wrapStreamExceptionsWith ServerDisconnected $
135-
Client.getResponseBodyChunk resp
135+
Client.getResponseBodyChunk' resp
136136
, _getTrailers = wrapStreamExceptionsWith ServerDisconnected $
137137
maybe [] fromHeaderTable <$>
138138
Client.getResponseTrailers resp

util/Network/GRPC/Util/Parser.hs

+68-26
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ module Network.GRPC.Util.Parser (
1010
, consumeExactly
1111
, getExactly
1212
-- * Execution
13+
, IsFinal
14+
, Leftover
15+
, ProcessResult(..)
1316
, processAll
14-
, processAllIO
1517
) where
1618

17-
import Control.Exception
1819
import Control.Monad
1920
import Data.Bifunctor
2021
import Data.Binary (Get)
@@ -158,33 +159,74 @@ getExactly len get =
158159
Execution
159160
-------------------------------------------------------------------------------}
160161

162+
type IsFinal = Bool
163+
type Leftover = Lazy.ByteString
164+
165+
data ProcessResult e b =
166+
-- | Parse error during processing
167+
ProcessError e
168+
169+
-- | Parsing succeeded (compare to 'ProcessedWithoutFinal')
170+
| ProcessedWithFinal b Leftover
171+
172+
-- | Parsing succeeded, but we did not recognize the final message on time
173+
--
174+
-- There are two ways that parsing can terminate: the final few chunks may
175+
-- look like this:
176+
--
177+
-- > chunk1 -- not marked final
178+
-- > chunk2 -- not marked final
179+
-- > chunk3 -- marked final
180+
--
181+
-- or like this:
182+
--
183+
-- > chunk1 -- not marked final
184+
-- > chunk2 -- not marked final
185+
-- > chunk3 -- not marked final
186+
-- > empty chunk -- marked final
187+
--
188+
-- In the former case, we know that we are processing the final message /as/
189+
-- we are processing it ('ProcessedFinal'); in the latter case, we realize
190+
-- this only after we receive the final empty chunk.
191+
| ProcessedWithoutFinal Leftover
192+
161193
-- | Process all incoming data
162194
--
163195
-- Returns any unprocessed data.
164-
processAll :: forall m e a.
196+
-- Also returns if we knew that the final result
197+
-- was in fact the final result when we received it (this may or may not be the
198+
-- case, depending on
199+
processAll :: forall m e a b.
165200
Monad m
166-
=> m Strict.ByteString -- ^ Get next chunk (empty indicates end of input)
167-
-> (a -> m ()) -- ^ Process single value
168-
-> Parser e a -- ^ Parser
169-
-> m (Either e Lazy.ByteString)
170-
processAll getChunk processOne parser =
201+
=> m (Strict.ByteString, IsFinal) -- ^ Get next chunk
202+
-> (a -> m ()) -- ^ Process value
203+
-> (a -> m b) -- ^ Process final value
204+
-> Parser e a -- ^ Parser
205+
-> m (ProcessResult e b)
206+
processAll getChunk processOne processFinal parser =
171207
go $ runParser parser nil
172208
where
173-
go :: Result e a -> m (Either e Lazy.ByteString)
174-
go (Failed err) = return $ Left err
175-
go (Done result bs') = processOne result >> go (runParser parser bs')
176-
go (NeedData parser' acc) = do
177-
bs <- getChunk
178-
if not (BS.Strict.null bs)
179-
then go $ runParser parser' (snoc acc bs)
180-
else return $ Right (toLazy acc)
181-
182-
-- | Wrapper around 'processAll' that throws errors as exceptions
183-
processAllIO :: forall e a.
184-
Exception e
185-
=> IO Strict.ByteString
186-
-> (a -> IO ())
187-
-> Parser e a
188-
-> IO Lazy.ByteString
189-
processAllIO getChunk processOne parser =
190-
processAll getChunk processOne parser >>= either throwIO return
209+
go :: Result e a -> m (ProcessResult e b)
210+
go (Failed err) = return $ ProcessError err
211+
go (Done a left) = processOne a >> go (runParser parser left)
212+
go (NeedData parser' left) = do
213+
(bs, isFinal) <- getChunk
214+
if not isFinal
215+
then go $ runParser parser' (left `snoc` bs)
216+
else goFinal [] $ runParser parser' (left `snoc` bs)
217+
218+
-- We have received the final chunk; extract all messages until we are done
219+
goFinal :: [a] -> Result e a -> m (ProcessResult e b)
220+
goFinal _ (Failed err) = return $ ProcessError err
221+
goFinal acc (Done a left) = goFinal (a:acc) $ runParser parser left
222+
goFinal acc (NeedData _ left) = do
223+
mb <- processLastFew (reverse acc)
224+
return $ case mb of
225+
Just b -> ProcessedWithFinal b $ toLazy left
226+
Nothing -> ProcessedWithoutFinal $ toLazy left
227+
228+
processLastFew :: [a] -> m (Maybe b)
229+
processLastFew [] = return Nothing
230+
processLastFew [a] = Just <$> processFinal a
231+
processLastFew (a:as) = processOne a >> processLastFew as
232+

0 commit comments

Comments
 (0)