Skip to content

Commit

Permalink
Fix some self-review notes
Browse files Browse the repository at this point in the history
  • Loading branch information
afharo committed Apr 1, 2021
1 parent 4685f63 commit b90fdec
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('EventBasedTelemetryService', () => {
test: { type: 'keyword', _meta: { description: 'Always OK because tests never fail' } },
},
})
).toThrowError('Channels can only be lifecycle step.');
).toThrowError('Channels can only be registered during the setup lifecycle step.');

// Checking the validator is registered for the channel
expect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class EventBasedTelemetryService {

public registerChannel(pluginName: string, channelOptions: EventChannelOptions) {
if (this.started) {
throw new Error(`Channels can only be lifecycle step.`);
throw new Error(`Channels can only be registered during the setup lifecycle step.`);
}
const { name: channelName, schema, quotaPercentage } = channelOptions;
this.logger.debug(`Registering channel "${channelName}" from plugin "${pluginName}"`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,20 @@ export class LeakyBucket {

private async sendIfFull(sender: HTTPSender) {
if (this.isFullQueue()) {
// To don't go over our "threshold bytes per second", we need to wait 1s before consuming from queues again.
// Doing a Promise.all because we don't want to add the sending time to the max_frequency_of_requests wait.
await Promise.all([
// To don't go over our "threshold bytes per second", we need to wait
// {max_frequency_of_requests} before consuming from queues again.
// Using Promise.allSettled because we don't want to add the sending
// time to the {max_frequency_of_requests} wait, but we still want to wait if we fail to send.
const [sendResults] = await Promise.allSettled([
this.send(sender),
new Promise((resolve) =>
setTimeout(resolve, this.config.max_frequency_of_requests.asMilliseconds())
),
]);

if (sendResults.status === 'rejected') {
throw sendResults.reason;
}
}
}

Expand Down

0 comments on commit b90fdec

Please sign in to comment.