Skip to content

Commit

Permalink
Use Promises for async api calls
Browse files Browse the repository at this point in the history
The first try at manually using spawn and channels worked, but if there
was an exception raised, it'd print a stack trace like this instead of
being handled nicely

    Unhandled exception in spawn:  (CB::Client::Error)
      from src/cb/client.cr:517:5 in 'exec'
      from src/cb/client.cr:484:5 in 'get'
      from src/cb/client.cr:144:5 in 'get_teams'
      from src/cb/completion.cr:126:22 in '->'
      from /usr/local/Cellar/crystal/1.4.1/src/fiber.cr:146:11 in 'run'
      from /usr/local/Cellar/crystal/1.4.1/src/fiber.cr:98:34 in '->'
    Unhandled exception in spawn:  (CB::Client::Error)
      from src/cb/client.cr:517:5 in 'exec'
      from src/cb/client.cr:484:5 in 'get'
      from src/cb/client.cr:144:5 in 'get_teams'
      from src/cb/client.cr:234:18 in 'get_clusters'
      from src/cb/completion.cr:128:22 in '->'
      from /usr/local/Cellar/crystal/1.4.1/src/fiber.cr:146:11 in 'run'
      from /usr/local/Cellar/crystal/1.4.1/src/fiber.cr:98:34 in '->'

The way to get around this would be to have the channel also be able to
take an exception, and resuce exceptions in the spawn block and send
them on the channel, then raise them in the main thread. This is a lot
of boilerplate work. I started writing a thing to do this for us, then
realized it was basically the promise pattern, and that someone else
probably has done that. I was a little hesitant to pull in this library
but it's not that large, and seems to do a good job.

So now when there is an exception we get the nicer output since this
takes care of all the steps to reraise exceptions in the right spot:

    error: 404 Not Found
           GET to /teams
           message: Resource 'account' with ID 'wef' couldn't be found.
           request_id: 26ac03b2-159e-4f0f-809e-84c474fe7b79

But then I realized that you can't actually call get clusters and get
teams at the same time, since you need to know the teams in order to
list their clusters. And this whole time for completion we were making
an extra call to get teams, since get_clusters with no args makes it's
own call to get teams. So I removed that unnecessary call from the
completion. The unhandeled exception problem though is a potential
problem in get_clusters, so it makes sense to use the new Promise
library there I think.
  • Loading branch information
will committed Jun 21, 2022
1 parent cafec65 commit cafe7b9
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 14 deletions.
6 changes: 5 additions & 1 deletion shard.lock
Expand Up @@ -20,9 +20,13 @@ shards:
git: https://github.com/will/crystal-pg.git
version: 0.26.0

promise:
git: https://github.com/spider-gazelle/promise.git
version: 3.0.0

raven:
git: https://github.com/sija/raven.cr.git
version: 1.9.1
version: 1.9.2

ssh2:
git: https://github.com/spider-gazelle/ssh2.cr.git
Expand Down
2 changes: 2 additions & 0 deletions shard.yml
Expand Up @@ -8,6 +8,8 @@ dependencies:
github: sija/raven.cr
ssh2:
github: spider-gazelle/ssh2.cr
promise:
github: spider-gazelle/promise

development_dependencies:
ameba:
Expand Down
2 changes: 1 addition & 1 deletion spec/cb/completion_spec.cr
@@ -1,7 +1,7 @@
require "../spec_helper"

private class CompletionTestClient < CB::Client
def get_clusters
def get_clusters(teams : Array(Team)? = nil)
[Cluster.new("abc", "def", "my cluster", [] of Cluster)]
end

Expand Down
7 changes: 2 additions & 5 deletions src/cb/client.cr
@@ -1,6 +1,7 @@
require "http/client"
require "json"
require "log"
require "promise"
require "../ext/stdlib_ext"

class CB::Client
Expand Down Expand Up @@ -235,11 +236,7 @@ class CB::Client
end

def get_clusters(teams : Array(Team))
ch = Channel(Array(Cluster)).new
clusters = [] of Cluster
teams.each { |t| spawn { ch.send get_clusters(t.id) } }
teams.size.times { clusters += ch.receive }
clusters.sort_by(&.name)
Promise.map(teams) { |t| get_clusters t.id }.get.flatten.sort_by!(&.name)
end

def get_clusters(team_id : String)
Expand Down
9 changes: 2 additions & 7 deletions src/cb/completion.cr
Expand Up @@ -122,13 +122,8 @@ class CB::Completion
end

def cluster_suggestions
tch = Channel(Array(CB::Client::Team)).new
spawn { tch.send client.get_teams }
cch = Channel(Array(CB::Client::Cluster)).new
spawn { cch.send client.get_clusters }

teams = tch.receive
clusters = cch.receive
teams = client.get_teams
clusters = client.get_clusters(teams)

clusters.map do |c|
team_name = teams.find { |t| t.id == c.team_id }.try(&.name) || "unknown_team"
Expand Down

0 comments on commit cafe7b9

Please sign in to comment.