Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No more console.log — Messaging.ts #192

Open
lautarodragan opened this issue Jun 28, 2018 · 0 comments
Open

No more console.log — Messaging.ts #192

lautarodragan opened this issue Jun 28, 2018 · 0 comments
Labels

Comments

@lautarodragan
Copy link
Member

lautarodragan commented Jun 28, 2018

Messaging.ts has six console.log calls — two in start and the remaining four in the business-specific consumePoetTimestampsDownloaded and consumeClaimsDownloaded functions.

The logs in the start function are needed in case of unexpected failure. We can sort them out by letting the consumer of the RabbitMQ client (in our case, all our module roots) pass the callbacks instead of the client having hard-coded ones.

It could look like this:

    this.amq = new AMQ(this.configuration.rabbitmqUrl)
    this.amq.onConnectionError(error => this.logger.error({ error }))
    this.amq.onConnectionClosed(() => this.logger.error({ }, 'RabbitMQ connection unexpectedly closed'))
    await this.messaging.start()

This code will be duplicated across modules, so we may want to create a Helper file to reduce code duplication a bit.

The calls in the business-specific functions act like warnings, akin to a compiler letting one know a function is being called with the incorrect arguments.

We need to decide whether we want to simply get rid of them or implement a mechanism for the consume* functions to provide feedback about dropped messages (such as a new argument to the function, a callback; or the Messaging itself taking it).

Could look like this:

    this.messaging = new Messaging(this.configuration.rabbitmqUrl)
    this.messaging.onDroppedMessage(droppedMessage => this.logger.warning({ droppedMessage }, 'RabbitMQ message dropped'))
    await this.messaging.start()

Ideally we want to implement this one after #66 to avoid introducing more noise.

@geoffturk geoffturk added this to the PML1 milestone Nov 21, 2018
@geoffturk geoffturk removed this from the PML1 milestone Nov 29, 2018
@geoffturk geoffturk added ct-z and removed ct-z labels Nov 29, 2018
@geoffturk geoffturk added this to the PML1 milestone Nov 29, 2018
@lautarodragan lautarodragan removed this from the PML1 milestone Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants