-
Notifications
You must be signed in to change notification settings - Fork 151
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
Make proto object verification give more descriptive error messages #1733
Make proto object verification give more descriptive error messages #1733
Conversation
common/protos/index.ts
Outdated
suggestReportToDataformTeam: boolean = false, | ||
showDocsLink: boolean = true |
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.
give me an Options
type so that these can be named and therefore readable at the callsite
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.
Good shout
common/protos/index.ts
Outdated
@@ -24,7 +28,8 @@ export interface IProtoClass<IProto, Proto> { | |||
// meaning that the type of fields cannot be verified; an int can be confused with a string. | |||
export function verifyObjectMatchesProto<Proto>( | |||
protoType: IProtoClass<any, Proto>, | |||
object: object | |||
object: object, | |||
options?: { suggestReportToDataformTeam?: boolean; showDocsLink?: boolean } |
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.
sorry, now I see that it looks like these options are effectively an enumeration - it essentially never makes sense for them both to be true; because suggestReportToDataformTeam
always takes precedence.
I'd suggest:
errorBehaviour?: enum ErrorBehaviour { SUGGEST_REPORTING_TO_DATAFORM_TEAM, SHOW_DOCS_LINK }
…ataform-co#1733) * Make proto object verification give better details * Fix error messages * Tidy * Add proto type URL to docs URL * Update options to be object based * Update to use enum for error behaviour
Given that we can't do proper type validation due to ProtobufJS, this is the next best thing that I can think of.