infinyon / fluvio

Lean and mean distributed stream processing system written in rust and web assembly. Alternative to Kafka + Flink in one.
https://www.fluvio.io/
Apache License 2.0
3.88k stars 491 forks source link

[Bug]: Misleading error on invalid topic name when using rust client. #3521

Closed gibbz00 closed 10 months ago

gibbz00 commented 1 year ago

What happened

When running CLI:

fluvio topic create dectest0f4d91a3a9104cf18c1c989a115812e9-some-very-long-topic-name

"Invalid argument: Invalid Topic name dectest0f4d91a3a9104cf18c1c989a115812e9-some-very-long-topic-name. Name exceeds max characters allowed 63."

When using rust the client:

`Result::unwrap()` on an `Err` value: FluvioAnyhow(Invalid topic name: topic name may only include lowercase letters (a-z), numbers (0-9), and hyphens (-).)

Expected behavior

Error message should point out exceeded character limit.

Possible starting point

https://github.com/infinyon/fluvio/blob/990892b09b60e11ed4d6cb4fb82557de8980ba1a/crates/fluvio-sc-schema/src/lib.rs#L61-L63

Environment:

jsat97 commented 1 year ago

@digikata Could you please look at these changes to see if they're ok ? https://github.com/jsat97/fluvio/tree/invalid-topic-name-error-rust-client I was trying to test it following this doc.

  1. ran "make" in fluvio root dir of my checked out repo.
  2. set aliases
  3. I see the following when I start the cluster ( flvd cluster start --local --develop) ✅ Local Cluster initialized 🖥️ Trying to connect to SC: localhost:9003 121 seconds elapsed / 2023-11-02T18:52:15.692823Z ERROR fluvio_cluster::start::common: fail to connect to sc at: localhost:9003 Timed out when waiting for SC service 4, helm list jai@ubuntu:~/fluvio$ helm list NAME NAMESPACE REVISION UPDATED STATUS CHART APP VERSION fluvio-sys default 1 2023-11-02 18:42:43.871846359 +0000 UTC deployed fluvio-sys-0.9.13 0.11.0-dev-1
  4. Where do I find the rust client as per the bug? The repo -- https://github.com/infinyon/flv-client-rust is archived. Do we have another ?
digikata commented 1 year ago

re: 4. The rust client in this case is the fluvio cli binary, so if you do a flvd topic create dectest0f4d91a3a9104cf18c1c989a115812e9-some-very-long-topic-name with your built cli binary, you should be able to cause the error as well as test a fix. (as long as you get a fluvio cluster working).

jsat97 commented 1 year ago

You have 2 errors in your reported output. The first one shows "Invalid Topic name" as well as the error on the limit of characters. the 2nd one shows the error on invalid characters. I thought these were produced differently (are cli and rust client the same ?) The error from fluvio appears to come from -- here The error is printed the same as before with my new binary (no changes to above code) jai@ubuntu:~/fluvio$ flvd topic create dectest0f4d91a3a9104cf18c1c989a115812e9-some-very-long-topic-name Invalid argument: Invalid Topic name dectest0f4d91a3a9104cf18c1c989a115812e9-some-very-long-topic-name. Name exceeds max characters allowed 63

digikata commented 1 year ago

re: 3. Depending on the kubernetes you are using locally, sometimes it's a little different to access the cluster. e.g. minikube, the node port is exposed directly, but not not all k8 installations or configs work with node port. So more diagnosis on the kubernetes setup might be needed.

At the master line right now, we have an alpha/beta non-kubernetes linked 'local' cluster config, so it might worth trying that out too as the topic bug of the issue might be reproducible with that configuration too. (support docs have not be updated for that yet though).

jsat97 commented 1 year ago

I dont have the cluster start problem in (3). Like I said last, I was able to bring up the cluster - but need clarification on the two error cases shown in the report -

  1. When running CLI:
  2. When using rust the client: Are these 2 different cases ? In (1), the error is reported correctly. so nothing to fix if this is the fluvio client. (2) -> how to repro this.
digikata commented 1 year ago

So technically there is a fluvio client crate which is something modular you can use to build into various other software - e.g. web servers or other backends, language bindings, etc. And there is the fluvio cli, which makes extensive use of that same fluvio-client crate.

If the error is reproduced differently in one context vs other, there is further diagnosis in this issue to figure out where the source of the error is - could be in the cli handling, but without digging further into it myself, I would guess it's deeper, in the client or possibly even in the cluster logic. or maybe the cli is providing error checking that the client is not.

sehz commented 1 year ago

I think this is due fact CLI is remapping error. Ideally, error from SC should be source of truth

jsat97 commented 1 year ago

I just saw your comments (they didn't show up until I refreshed the page!)

If the error is reproduced differently in one context vs other,

understand. The fluvio command (if this is called the "client") is giving the correct output (error does say > 63 chars) The 2nd case (cli ? ) is the one I am unable to reproduce. @sehz

I think this is due fact CLI is remapping error.

The mapping is on the line I've shown above (create.rs) There is apparently another case where validate_resource_name is called and the error returned is mapped incorrectly. I may have already fixed this in my PR but don't know how to test it.

github-actions[bot] commented 10 months ago

Stale issue message