taoensso / sente

Realtime web comms library for Clojure/Script
https://www.taoensso.com/sente
Eclipse Public License 1.0
1.74k stars 193 forks source link

Bad package error when sending a complicated keyword to the client #334

Closed pieterbreed closed 5 years ago

pieterbreed commented 5 years ago

I'm sending this as event data to clients from the server:

{
:docker-registry.zoona.io/tachyon/monolith-comms/launcher 
{:ref "master", :project "tachyon/monolith-comms"}, 
:docker-registry.zoona.io/ops-engineering/kanaloa/launcher 
{:ref "master", :project "ops-engineering/kanaloa"}}

The client receives the data event, but I get this DEBUG message and error:

DEBUG [taoensso.sente:210] - Bad package: [[:kanaloa/dependencies {"about/page" {:docker-registry.zoona.io/tachyon/monolith-comms/launcher {:ref "master", :project "tachyon/monolith-comms"}, :docker-registry.zoona.io/ops-engineering/kanaloa/launcher {:ref "master", :project "ops-engineering/kanaloa"}}}]] (#error {:message "Invalid keyword: docker-registry.zoona.io/tachyon/monolith-comms/launcher.", :data {:type :reader-exception, :ex-kind :reader-error}}) taoensso.timbre.appenders.core.js:157:8
Error: Invalid event

I'm not entirely sure if this is a bug; if this is a bug in sente or in encore. I think I can simplify the keyword a little bit, maybe that will stop the error from showing up on my side, but I suspect there might be something going on here that needs fixing.

theasp commented 5 years ago

I don't think having multiple / in a keyword is valid, despite the fact that keyword lets you make it. According to the EDN docs (https://github.com/edn-format/edn#keywords and https://github.com/edn-format/edn#symbols):

/ has special meaning in symbols. It can be used once only in the middle of a symbol to separate the prefix (often a namespace) from the name, e.g. my-namespace/foo. / by itself is a legal symbol, but otherwise neither the prefix nor the name part can be empty when the symbol contains /.

You could convert your keywords to strings before serializing, and then keyword it later.

danielcompton commented 5 years ago

Yeah, these aren't syntactically valid keywords. Unfortunately, Clojure lets you construct keywords from strings which aren't able to be printed/round-tripped even by itself. In your case, you need to send those URLs as strings, not keywords.