Skip to content
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

[analytics] add error field to feedback event #10455

Closed
wants to merge 1 commit into from

Conversation

laushinka
Copy link
Contributor

@laushinka laushinka commented Jun 3, 2022

Description

[@jldec updated Aug 17, 2022]
The intent of this PR was to add the rendered error message along with the feedback tracking event, however, the object used in this implementation does not actually include the rendered error message, because that is derived in JSX.

With error Without error
Screenshot 2022-06-03 at 14 18 12 Screenshot 2022-06-03 at 15 26 14

Related Issue(s)

Fixes #10402

How to test

  1. Trigger an error when creating a workspace (steps here).
  2. Visit Segment and filter for feedback_submitted.
  3. See the error_rendered body.
  4. Send a non-error feedback and see no error_rendered field on Segment.

Release Notes

NONE
  • /werft with-preview
  • /werft analytics=segment|TEZnsG4QbLSxLfHfNieLYGF4cDwyFWoe

@laushinka
Copy link
Contributor Author

laushinka commented Jun 3, 2022

/werft run

👍 started the job as gitpod-build-laushinka-send-error-message-with-10402.1
(with .werft/ from main)

@jldec
Copy link
Contributor

jldec commented Jun 7, 2022

I think we should try to include the actual error text in the feedback report

Information about the context would be good too e.g. to tell the difference between 404 errors in say
a. starting a workspace from a repo URL context
vs.
b. starting a workspace from a snapshot URL

@laushinka laushinka force-pushed the laushinka/send-error-message-with-10402 branch from 4482529 to b27e593 Compare June 7, 2022 10:15
@stale
Copy link

stale bot commented Jun 18, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Jun 18, 2022
@stale stale bot closed this Jun 24, 2022
@jldec jldec reopened this Jul 15, 2022
@stale stale bot removed the meta: stale This issue/PR is stale and will be closed soon label Jul 15, 2022
@jldec jldec force-pushed the laushinka/send-error-message-with-10402 branch from b27e593 to 811ca8e Compare July 15, 2022 16:12
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-laushinka-send-error-message-with-10402.4 because the annotations in the pull request description changed
(with .werft/ from main)

@jldec
Copy link
Contributor

jldec commented Jul 15, 2022

@laushinka I rebased and updated the werft flags in the description

@jldec
Copy link
Contributor

jldec commented Jul 15, 2022

Testing error reporting on workspace start with private repo

Screenshot 2022-07-15 at 18 07 22 Screenshot 2022-07-15 at 18 08 26

@stale
Copy link

stale bot commented Jul 30, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Jul 30, 2022
@stale stale bot closed this Aug 10, 2022
@jldec jldec reopened this Aug 10, 2022
@jldec jldec added meta: never-stale This issue can never become stale and removed meta: stale This issue/PR is stale and will be closed soon labels Aug 10, 2022
@jldec
Copy link
Contributor

jldec commented Aug 10, 2022

This implementation works, but it is incomplete because the error message text is not included with the feedback.
We either have to grab an image, or (better), extract the error message text as a string.

Currently, error handling implementations e.g. in CreateWorkspace.tsx render errors directly to HTML, instead of starting with plaintext first, and then rendering it.

@laushinka laushinka force-pushed the laushinka/send-error-message-with-10402 branch from 9392116 to 6f1e5db Compare September 15, 2022 10:05
@roboquat roboquat added size/S and removed size/XS labels Sep 15, 2022
@laushinka
Copy link
Contributor Author

Closing because too stale. Picked up here.

@laushinka laushinka closed this Sep 15, 2022
@laushinka laushinka deleted the laushinka/send-error-message-with-10402 branch October 11, 2022 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send error message with feedback
3 participants