-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add definitions for a module erroring #839
base: main
Are you sure you want to change the base?
Conversation
7b274d1
to
dc421bc
Compare
dc421bc
to
f139ff6
Compare
I suggest building on toolkit_openModule from #835 and making these
These become more generalized than "web views" (suggestion is non-blocking) |
@@ -7138,6 +7138,26 @@ | |||
"type": "result" | |||
} | |||
] | |||
}, | |||
{ | |||
"name": "webview_error", |
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 is this needed? Every event can store result
, reason
, reasonDesc
. Errors should not be modeled as separate events, they should be modeled as existing events with result=Failed
+ reason
.
E.g. in this case webview_load could be such an event. If failure can occur after load, and we want a generic "webview interaction" event, then let's consider that instead. E.g. webview_click
, webview_submit
, etc.
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.
webview_error in this case is an generic error that can get thrown inside of mynah ui. Not sure if you can tie it directly to a chatMessage or anything since it can appear whenever: https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/amazonq/webview/ui/main.ts#L26
In the case of webview_load, thats how we do it. If the webview fails to load we set the reason description and result failed
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.
see #839 (comment)
+1 Though Instead of dedicated "error" events, model actual events which can fail (result=Failed). |
Does In VSCode it's goal is to track: |
That could make sense. Mentioning "Example: window.addEventListener('error')" in the description could help too. OTOH, if we want a generic thing that captures unknown webview events, can we not name it |
Problem
We have no way to track errors that occur within a module that occur outside of a regular telemetry execution scope. E.g. listening on a window for errors via
window.addEventListener('error')
Solution
toolkit_errorModule
which is called when an unexpected error occurs outside of regular telemetry messagesLicense
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.