v0.2.0

[alexluong]

Adds support for:

Listening on multiple sources #70

$ hookdeck listen 1234 source-1,source-2

$ hookdeck listen 1234 '*'

Notes: Based on the way our CLI processes argument, when running hookdeck listen 1234 *, it will pass every files in the directory as arguments, causing a validation issue. Therefore, we need to do hookdeck listen 1234 '*' for it to work. Curious what yall think about this. It may make more sense to change it to a flag like hookdeck listen 1234 --all or hookdeck listen 1234 --all-source.

Defaults for optional listen command connection and path arguments

This removes the interactivity prompts for the connection "label" and "path".

See #92.

Ability to set the CLI Path using --cli-path flag

See #92.

[leggetter]

@alexluong I much prefer:

hookdeck listen 1234 --all-sources

Over

hookdeck listen 1234 '*'

I also think we should support the following:

hookdeck listen 1234 "source-1 source-2 source-3"

hookdeck listen 1234 "source-1, source-2, source-3"

[alexbouchardd]

Can we look into the root cause as to why * itself doesn't work? Shouldn't that be something we can change?

hookdeck listen 1234 --all-sources is inconsistent with the params semantic and looses the ability to query connections since you no longer have a source param

[alexbouchardd]

Because we use connection query and limit it to 1 connection, in the sources section we still show all sources. Should we only show sources that we're currently proxying?

👍 Make sense to only display the one that can generate events

[alexluong]

Can we look into the root cause as to why * itself doesn't work? Shouldn't that be something we can change?

Yes, this is on my TODO list. I think it has something to do with the Cobra package or how Go flags work but haven't looked further. It's pretty hard to Google tho 😅

[leggetter]

See https://stackoverflow.com/a/2755803/39904 for the reasoning behind the *.

User's can do \* or set some command prompt flag prior to using the CLI. We're not going to easily get the behavior we're looking for.

[alexluong]

Thanks @leggetter! How would you suggest we go about this then?

[alexluong]

@leggetter @alexbouchardd I pushed some new commits that addressed your feedback, just have the * issue left.

[alexbouchardd]

Right * is out of the question then. I'm leaning toward '*' since it preserves the argument position over a flag. It sounds like \* would also work...

[alexluong]

Right is out of the question then. I'm leaning toward '' since it preserves the argument position over a flag. It sounds like * would also work...

The code should already work with both of these!

@leggetter please feel free to QA and merge whenever you're ready. I'm happy with where things are so we should be good to move forward here.

[leggetter]

@alexluong upon running:

./hookdeck-cli listen 3030 '*'                                                                                                                                                                                                                                                                                              

I'm seeing:

Dashboard
👉 Inspect and replay events: https://dashboard.hookdeck.com?team_id={id}

Sources
🔌 X URL: https://hkdk.events/xxx
🔌 my-example-source-3 URL: https://hkdk.events/xxx
🔌 openai-callback URL: https://hkdk.events/xxx
🔌 open-ai-api URL: https://hkdk.events/xxx
🔌 batch-input URL: https://hkdk.events/xxx
🔌 vonage-sms-prod URL: https://hkdk.events/xxx
🔌 shopify-events-prod URL: https://hkdk.events/xxx
🔌 XML-inbound URL: https://hkdk.events/xxx

Connections
X -> omSGu^ forwarding to /local/path/webhooks
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x10337d7a4]

goroutine 1 [running]:
github.com/hookdeck/hookdeck-cli/pkg/listen.printConnections(0x103564d28?, {0x140001d3480, 0xf, 0x0?})
    /Users/leggetter/hookdeck/git/hookdeck-cli/pkg/listen/printer.go:40 +0x94
github.com/hookdeck/hookdeck-cli/pkg/listen.Listen(0x140001dc2d0, {0x16ce73573?, 0xd?}, {0x0, 0x0}, {0x38?}, 0x103875a00)
    /Users/leggetter/hookdeck/git/hookdeck-cli/pkg/listen/listen.go:83 +0x3e4
github.com/hookdeck/hookdeck-cli/pkg/cmd.(*listenCmd).runListenCmd(0x140001be800, 0x0?, {0x140001be980, 0x16ce7356e?, 0x103144ea0?})
    /Users/leggetter/hookdeck/git/hookdeck-cli/pkg/cmd/listen.go:115 +0x184
github.com/spf13/cobra.(*Command).execute(0x14000248848, {0x140001be940, 0x2, 0x2})
    /Users/leggetter/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:842 +0x568
github.com/spf13/cobra.(*Command).ExecuteC(0x10386fe20)
    /Users/leggetter/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:950 +0x310
github.com/spf13/cobra.(*Command).Execute(...)
    /Users/leggetter/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:887
github.com/hookdeck/hookdeck-cli/pkg/cmd.Execute()
    /Users/leggetter/hookdeck/git/hookdeck-cli/pkg/cmd/root.go:46 +0x2c
main.main()
    /Users/leggetter/hookdeck/git/hookdeck-cli/main.go:21 +0x1c

[alexluong]

Ahh thanks for catching that @leggetter. It's a bit of a data thing so in my local test connections setup I didn't run into it.

It's a bug that I missed from the refactor yesterday. We needed to filter to make sure we only worked with connections leading to a CLI destination. It was there but in the refactor yesterday I accidentally removed it and didn't catch it in my QA.

Pushed a fix now, please take a look again and let me know if that works!

[leggetter]

@alexluong Ok, so if multiple sources or '*' is passed, we'll only listen if the source already has a connected Destination of type CLI?

But if someone calls listen with a single source name:

hookdeck listen {PORT} {SOURCE_NAME}

Then a connection is created?

Is it too complicated to prompt for a CLI Path when the source isn't of type CLI?

[alexluong]

@leggetter yes, that's correct. I believe this is the result of a discussion with @alexbouchardd on Slack.

Is it too complicated to prompt for a CLI Path when the source isn't of type CLI?

When listening to multiple sources, there's a chance there are many sources without a connection to a CLI destination. That could also be intentional, like production sources that are never proxied. Therefore, it may not make sense to prompt for every single one of those. I hope this explains the thought process behind it but if I don't understand you correctly, please feel free to elaborate.

[leggetter]

@alexluong see the following:

Naming a single Source that doesn't have a CLI destination will prompt for a path and will dynamically set up a Connection.

./hookdeck-cli listen 3030 'batch-input'
? What path should the events be forwarded to (ie: /webhooks)?

vs. naming two Sources where one doesn't have a CLI Destination ignores it.

./hookdeck-cli listen 3030 'batch-input,X'

Dashboard
👉 Inspect and replay events: https://dashboard.hookdeck.com?team_id=tm_gCebUjh8rk0c

Sources
🔌 X URL: https://hkdk.events/j55l4yc9aojy2i

Connections
X -> omSGu^ forwarding to /local/path/webhooks

> Ready! (^C to quit)

I really don't like the DX of this as it's inconsistent.

I think it's reasonable for hookdeck listen '*' to only connect to Sources that have a CLI Destination defined. But if someone has specifically listed a Source and the CLI then just ignores it, it's confusing.

At the very least, we should log something instructional for the developer.

e.g. if the developer uses * then we should inform them of the restriction we're enforcing.

Listening for events on the first 10 Sources that have Connections with CLI Destinations

For the above, we may need to update the message since we only try Sources available in the first 10 or to update the functionality we've implemented since it's quite hard to describe the current functionality.

e.g. if the developer uses source-1,source-2

Listening for events on Sources that have Connections with CLI Destinations

Great that you've caught the situation where someone specifies more than 10 sources 👍

[alexluong]

Hi @leggetter, is this what you're looking for?

If the user only sends 1 source then we won't print anything here. Pushed the commit for this, please take a look and let me know if you'd like to make further changes here.

I really don't like the DX of this as it's inconsistent.

While I do see where you're coming from, I think it makes sense. We basically have 2 modes with 2 vastly different experiences: single source mode and multi source mode. For single source, our mindset is doing what it takes to help them with their connection so they can start using Hookdeck. For multi source, because we can't assist individually in case there are multiple sources with "issues", we choose to ignore those sources and just focus on the relevant ones.

[leggetter]

@alexluong I still think this will confuse people, but we can publish and share see what feedback we have.

One last question. When I run hookdeck listen 3030 '*' I'd expect to see:

• X
• vonage-local
• twilio-local
• cli

All connected. However, I only get the first two. Why is that? I can see we have a temp limit on the sources we query when it feels like we should impose that on the number of sources we can listen to.

[alexluong]

@leggetter That's fair. What do you think is a better DX though? We can potentially have a more "elaborate" mode where we showcase the process:

• query 10 sources, here are the results...
• query connections, here are the results...
• of those connections, here are the connections pointing to a CLI destination
• get sources linked to those connections
• ... the rest that you can see

All connected. However, I only get the first two. Why is that?

The current logic is we query 10 sources with Hookdeck's default sort & filter logic. Most likely that request yielded you the 10 sources, 8 of which without a CLI destination. Unfortunately, we can't query something like "give me 10 sources with connections to CLI destination", I don't think.

[leggetter]

@alexluong, is there any reason we can't query 100 sources but restrict the number of Sources we connect to the first 10 we find?

Is the problem here that we'll potentially be making 101 requests? 1 to request the sources and potentially 100 to filter through the connections with CLI destinations.

[alexluong]

@leggetter yes, that is essentially the case. The only difference is we'll make 101 requests, 100 request to query for sources (because we can only send 1 name) and 1 request to query for connections.

[leggetter]

@alexluong - would you mind giving the updates on the merge of #92 into this a review and update?

[alexluong]

@leggetter yes, I'll take a deeper look tomorrow. Is there anything you'd like me to update?

[leggetter]

@leggetter yes, I'll take a deeper look tomorrow. Is there anything you'd like me to update?

Nothing functionally.

So:

1. A once-over of the code. I'm not a Go developer so I may well have added code that smells weird 😄
2. Some general Q&A would also be great

[alexluong]

@leggetter Got it, so moreso code polishing & making sure there's no issue and just keep the functionality the same as laid out in #92.

[leggetter]

@leggetter Got it, so moreso code polishing & making sure there's no issue and just keep the functionality the same as laid out in #92.

Yes, 👍 Thank you!