Conversation
raupie
force-pushed
the
nodejs-rabbitmq-example
branch
from
87f5b50 to
62b188b
Compare
|
squashed and rebased |
|
You also need to add RabbitMQ to the list of services in |
raupie
force-pushed
the
nodejs-rabbitmq-example
branch
from
62b188b to
d214ff6
Compare
|
Code looks fine, but stuck build referred to support. |
Update NPM packages for security releases.
| @@ -0,0 +1,30 @@ | |||
| const amqp = require('amqplib/callback_api'); | |||
There was a problem hiding this comment.
You are using the Promise-based API but this imports the callback one.
The Promise-based API is the default one so you can do const amqp = require('amqplib');
| let message = 'Deploy Friday!'; | ||
|
|
||
| await channel.assertQueue(queue, {durable: false}); | ||
| await channel.sendToQueue(queue, Buffer.from(message)) |
There was a problem hiding this comment.
sendToQueue behaves like node's Stream.writable: it returns a boolean, not a Promise. You don't have anything to await here.
You can ignore the returned value though.
JavaScript lets you write anything and try to make sense of it (with all the implications this can have...). That's why nothing complains about the fact that you're awaiting for a primitive value when it would only make sense for a Promise.
| await channel.assertQueue(queue, {durable: false}); | ||
| await channel.sendToQueue(queue, Buffer.from(message)) | ||
|
|
||
| let output = 'Message sent: ' + queue + ' ' + message; |
There was a problem hiding this comment.
You don't mutate any variable so your lets can be consts.
This applies to all the let in the function.
|
|
||
| let output = 'Message sent: ' + queue + ' ' + message; | ||
|
|
||
| return output; |
There was a problem hiding this comment.
It would be nice to consume the message before returning to be consistent with the other language examples.
| let output = 'Message sent: ' + queue + ' ' + message; | ||
|
|
||
| return output; | ||
| } catch (error) { |
There was a problem hiding this comment.
Everything is supposed to run just fine in this example, so you might want to get rid of the try/catch just to remove some noise around the code example.
I don't feel strongly about this though.
Just for the sake of completion, async/await is good but not always clearer. It feels like writing synchronous code when things are async so it looks simple and familiar but has some caveats e.g. control flow and error handling. This catch here would catch any error happening in the try block so it makes it hard to have a good error handling mechanism (What broke? The connection? A non-promise-related exception got thrown? Something else?).
It's okay to use a .then(...) approach if that makes things simpler to reason about. Whether or not things are actually "simpler" is pretty subjective though.
|
Been so long, I forget what I was trying to do. Burning down and starting over... |
|
New PR here #48 |
4 participants
No description provided.