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
Amazon Q: Added telemetry metrics and modified customer facing messages for Q Codescans. #4951
Conversation
packages/toolkit/.changes/next-release/Bug Fix-ebe770da-054d-43e7-8141-cd32b6159e3e.json
Outdated
Show resolved
Hide resolved
packages/toolkit/.changes/next-release/Bug Fix-ebe770da-054d-43e7-8141-cd32b6159e3e.json
Outdated
Show resolved
Hide resolved
CodeScanJobFailedError = 'CodeScanJobFailedError', | ||
} | ||
|
||
export const mapErrorToCustomerFacingMessage: Record<ErrorCodes, string> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just store the "customer facing message" on the error? there is way too much over-abstraction and indirection in this PR
} | ||
} | ||
|
||
export class ProjectSizeExceededError extends ToolkitError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are creating custom classes for every error, yet not using the features of a class. the class could store the "customer facing message", and possibly could also avoid the enums and "mapping" indirection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created a new SecurityScanError
class which extends ToolkitError
to add a new field customerFacingMessage
. This will work without ErrorCodes
enum and mapErrorToCustomerFacingMessage
.
export const DefaultCodeScanErrorMessage = | ||
'Amazon Q encountered an error while scanning for security issues. Try again later.' | ||
|
||
export const FileSizeExceededErrorMessage = `Amazon Q: The selected file exceeds the input artifact limit. Try again with a smaller file. For more information about scan limits, see the [Amazon Q documentation](https://docs.aws.amazon.com/amazonq/latest/qdeveloper-ug/security-scans.html#quotas).` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these could be stored directly on the Error classes.
Problem
Solution
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.