🔊 Add better logging when loading the Shoryuken environment

[danielvdao]

Context

When running shoryuken, we ran into an issue where the queue didn't exist. In theory this is true because we don't have permission to access the queue, but the suggestion to create a queue was a bit mis-leading.

More of a suggestion if anything since I noticed we were struggling and maybe it could help others as well! Ultimately the solution was stumbled upon in a SO post and some prior knowledge / experience.

Issue link: #692

[danielvdao]

@cjlarose thanks for triggering the job. I tried to change the HEREDOC to match, apparently it doesn't like the ~ for that version of Rails? Having trouble reproducing it locally even though I tried running bundle exec appraisal rails_4_2 rake rails:spec, so maybe it's my ruby version.

[danielvdao]

Yeah, I'm running ruby 3 locally. Having trouble trying to drop down to ruby 2.2.10 which is what's being used to run, but my suspicion is that the HEREDOC syntax must've changed based on these docs.

[cjlarose]

That makes sense. I think the "sqiggly" heredoc was introduced in Ruby 2.3.

I like this change; my own Ops team is constantly reassuring our devs that their queues exist when they get this error message; it's almost always a permissions problem.

I think the error message is formatted a little weird though.

In master,

/Users/chris.larose/workspace/ruby-shoryuken/shoryuken/lib/shoryuken/environment_loader.rb:163:in `validate_queues': The specified queue(s) dev_cjlarose_accounts_receivable.fifo do not exist. (ArgumentError)
Try 'shoryuken sqs create QUEUE-NAME' for creating a queue with default settings

This branch:

/Users/chris.larose/workspace/ruby-shoryuken/shoryuken/lib/shoryuken/environment_loader.rb:169:in `validate_queues':         "The specified queue(s) dev_cjlarose_accounts_receivable.fifo do not exist. (ArgumentError)
         Try 'shoryuken sqs create QUEUE-NAME' for creating a queue with default settings.
         It's also possible that you don't have permission to access the specified queues."

It'd be great to omit the quotes and remove the leading whitespace. Also, if you could add a test to spec/shoryuken/environment_loader_spec.rb to assert that this error message is raised, that'd be awesome!

[danielvdao]

@cjlarose you got it, let me take care of that - I feel like an archaeologist trying to figure out how the squiggly works.

[cjlarose]

Haha yeah. Sorry about the old Ruby support! Ruby 2.4 and older are actually EOL already according to https://www.ruby-lang.org/en/downloads/branches/

I think in the next major shoryuken version, we'll drop support for some of the older versions to make maintenance easier.

[danielvdao]

@cjlarose

Sorry about the old Ruby support!

Not at all :), thank you for maintaining this gem - the effort is much appreciated.

As for the tests, I believe I've added them, but let me know. It took a bit of reverse-engineering as I'm not too familiar with the nitty gritty of what's going on. Let me know if there's things I can do better to conform with the repo style.

Apologies for the delay, was working on some of my day job stuff as well and hopping back <> forth.

[danielvdao]

Ha, the ~ operator is actually used to deal with indentation. The suggested / cleanest solution I could find was to use #gsub. I know it can be expensive though (since it's re-creating string objects), but this doesn't seem like a potential blocker since this command is invoked on load versus runtime.

[cjlarose]

This solution looks good! Like you said, this isn't a hot path, so the performance won't matter much.

[danielvdao]

I guess looking at this, I can do something else similar to - https://stackoverflow.com/a/54314792. It could be better to not depend on an underlying API which is susceptible to change in the specs, otherwise if AWS SDK changes number of parameters, etc. to the Aws::ServiceError class, this test would break. WDYT? @cjlarose

[cjlarose]

I didn't know about stub_const but it looks like it might be worth trying here!

[cjlarose]

Changes look good! Lemme know if you wanna try the stub_const approach.

[danielvdao]

@cjlarose haha I'm not too married to it and I think this solution is good enough (gave it a shot and it seemed a little trickier than I thought). Unfortunately I have other things to attend to as well so don't want to spend too much time on this 😞 Thank you for taking a look at this!

[cjlarose]

Awesome. Thanks for adding this!