Repository navigation
feat: add custom metric functionality - #39
Conversation
| async cmdIncrementMetric(name, value = 1) { | ||
| if (this.debug) console.log('in cmdIncrementMetric') | ||
|
|
||
| if (typeof name !== 'string' || name.length == 0) { |
There was a problem hiding this comment.
Can you move the duplicated statements in these methods to a validation method? Should this also be > 0? We should use should use strict equality checks for consistency.
There was a problem hiding this comment.
Isn't much duplication that we can get rid of here especially since we would have to pass in these strings and values that are different across all of these. > 0 would be for a positive check, this is a negative check so == 0 is correct
| if (this.debug) console.log('in cmdIncrementMetric') | ||
|
|
||
| if (typeof name !== 'string' || name.length == 0) { | ||
| throw 'IncrementMetric: metric name must be set!'; |
There was a problem hiding this comment.
Can you use Error('message') to preserve stack trace?
There was a problem hiding this comment.
Valid. My only hesitation with this is that there is 105 other occurrences throughout in the file that should also change to match the pattern
|
|
||
| // Send command to the engine | ||
| const result = await this.queueCommands.sendMessage(command); | ||
| if (result.success) |
There was a problem hiding this comment.
Should this throw here so the caller can handle the errors for all these? Otherwise it requires explicit check of this string which might be missed.
There was a problem hiding this comment.
This explicit check already exists with the callers of all other functions in the file that have this pattern. If we change that for these functions we'll have 2 different patterns
Issue #, if available: https://taskei.amazon.dev/tasks/Wickr-25457
Description of changes:
Add custom bot metric functionality. Three functions for bot integrations to publish their own metrics to the bot engine.