feat(browser): Add `captureUserFeedback` #7729

krystofwoldrich · 2023-04-04T13:15:05Z

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

If you've added code that should be tested, please add tests.
Ensure your code lints and the test suite passes (yarn lint) & (yarn test).
closes: Add an API for User Feedback #6252

size-limit report 📦

Path	Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified)	20.89 KB (+0.8% 🔺)
@sentry/browser - ES5 CDN Bundle (minified)	65.29 KB (+0.84% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)	19.44 KB (+0.83% 🔺)
@sentry/browser - ES6 CDN Bundle (minified)	57.75 KB (+0.91% 🔺)
@sentry/browser - Webpack (gzipped + minified)	21.06 KB (+1.07% 🔺)
@sentry/browser - Webpack (minified)	68.73 KB (+0.79% 🔺)
@sentry/react - Webpack (gzipped + minified)	21.09 KB (+1.06% 🔺)
@sentry/nextjs Client - Webpack (gzipped + minified)	48.87 KB (+0.38% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)	28.45 KB (+0.51% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)	26.68 KB (+0.56% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified)	44.88 KB (+0.01% 🔺)
@sentry/replay - Webpack (gzipped + minified)	38.86 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified)	63.76 KB (+0.29% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified)	56.8 KB (+0.32% 🔺)

…ck-api

AbhiPrasad · 2023-04-05T09:52:34Z

packages/browser/src/userfeedback.ts

+    tunnel: client.getOptions().tunnel,
+  });
+
+  void transport.send(envelope);


it's times like this I wish client.sendEnvelope was exposed - @lforst do you think we should just expose it?

m: We can only call transport.send if client.getOptions().enabled !== false

m: We can only call transport.send if client.getOptions().enabled !== false

I have a feeling this should be checked in a more central location. Doesn't have to be this PR.

it's times like this I wish client.sendEnvelope was exposed - @lforst do you think we should just expose it?

Probably? It seems safer that this whole chain of grabbing stuff off the hub that may or may not be there. But honestly sendEnvelope seems a bit too low-level to just expose it on a pretty public place.

I vote we take the bundle size hit and go ahead and implement user feedback like we implemented sessions, errors, and transactions just to stay consistent - meaning we put it as a captureUserFeedback() method on the (browser-)client.

Sounds good to me @krystofwoldrich - sorry for making you revert your work, but let's just add the methods to the browser client.

I don't mind either, but the stand-alone function is a bit easier for reuse in other JS SDKs,

if we implement it on the Browser client we have to export the user feedback envelope creator for other JS-based SDKs,

if we would implement it as a standalone function other JS SDKs can use the method right away.

Both electron and react native wrap browser - so it's only the node SDKs that won't have access to this, and I think that is fine.

Okay, that's fair not that many SDKs, just a note RN doesn't wrap Browser.

packages/browser/src/userfeedback.ts

lforst · 2023-04-05T10:05:46Z

packages/browser/src/userfeedback.ts

+    tunnel: client.getOptions().tunnel,
+  });
+
+  void transport.send(envelope);


m: We can only call transport.send if client.getOptions().enabled !== false

I have a feeling this should be checked in a more central location. Doesn't have to be this PR.

lforst · 2023-04-05T10:09:52Z

packages/browser/src/userfeedback.ts

+    tunnel: client.getOptions().tunnel,
+  });
+
+  void transport.send(envelope);


it's times like this I wish client.sendEnvelope was exposed - @lforst do you think we should just expose it?

Probably? It seems safer that this whole chain of grabbing stuff off the hub that may or may not be there. But honestly sendEnvelope seems a bit too low-level to just expose it on a pretty public place.

I vote we take the bundle size hit and go ahead and implement user feedback like we implemented sessions, errors, and transactions just to stay consistent - meaning we put it as a captureUserFeedback() method on the (browser-)client.

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>

This reverts commit e295944.

fliespl · 2023-09-27T21:48:01Z

How about adding envelope promises here and returning it?

Without it it's not possible to handle api error (at least to my knowledge).

lforst · 2023-09-28T07:57:54Z

@fliespl Would you mind elaborating?

fliespl · 2023-09-28T09:40:53Z

Currently if I want to build custom user feedback and send request using:

        const userFeedback = {
            event_id: eventId,
            name: data.get('name'),
            email: data.get('email'),
            comments: data.get('comments')
        };

        Sentry.captureUserFeedback(userFeedback);

there is no way to find out if it was successfully sent or not.

I was hoping for this function to reurn fetch/XHR promise that allows to do specific operation depending on success/failure.

lforst · 2023-09-29T08:29:45Z

@fliespl There are technical reasons for us not to expose whether an event (which user feedback is) was successfully sent or not. The SDK follows a fire-and-forget policy for all telemetry. The storing of events happens asynchronously and could be slow. The ingestion endpoint itself doesn't know whether a request was successfully sent so it will always return a success response with the exception of rate limiting.

We will likely not change anything about this.

fliespl · 2023-09-29T09:01:31Z

@lforst understood. I was hoping I could match: showReportDialog which shows error in case feedback is not sent, but that's because it operates differently I guess.

This adds an API to the Sentry Python SDK that captures user feedback via envelope. This is implemented very similiarly to how it is done for the JavaScript SDK, see getsentry/sentry-javascript#7729. Fixes getsentryGH-1064

feat(browser): Add captureUserFeedback

1407448

krystofwoldrich requested review from AbhiPrasad, Lms24 and lforst April 4, 2023 21:03

krystofwoldrich added 5 commits April 5, 2023 11:07

Add integration test

314b615

Merge remote-tracking branch 'origin/develop' into kw-add-user-feedba…

a0394a0

…ck-api

Make user feedback tree shakable

e295944

Fix possibly undefined client

6b8b10c

Refactor debug logs

f29a85f

AbhiPrasad reviewed Apr 5, 2023

View reviewed changes

lforst reviewed Apr 5, 2023

View reviewed changes

krystofwoldrich and others added 3 commits April 5, 2023 13:31

Apply suggestions from code review

2c5aae5

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>

Revert "Make user feedback tree shakable"

631d858

This reverts commit e295944.

Export user feedback envelope creator

d30527c

krystofwoldrich requested review from lforst and AbhiPrasad April 5, 2023 11:49

Add isEnabled check

31c54c8

AbhiPrasad approved these changes Apr 5, 2023

View reviewed changes

Lms24 merged commit c90a60f into develop Apr 5, 2023
55 checks passed

Lms24 deleted the kw-add-user-feedback-api branch April 5, 2023 12:44

theCapypara mentioned this pull request Oct 11, 2023

Add util to send user feedback getsentry/sentry-python#1064

Closed

theCapypara mentioned this pull request Oct 13, 2023

feat(api): Support capturing user feedback getsentry/sentry-python#2442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(browser): Add `captureUserFeedback` #7729

feat(browser): Add `captureUserFeedback` #7729

krystofwoldrich commented Apr 4, 2023 •

edited

github-actions bot commented Apr 4, 2023 •

edited

AbhiPrasad Apr 5, 2023 •

edited

lforst Apr 5, 2023

lforst Apr 5, 2023

AbhiPrasad Apr 5, 2023

krystofwoldrich Apr 5, 2023

AbhiPrasad Apr 5, 2023

krystofwoldrich Apr 5, 2023

lforst Apr 5, 2023

lforst Apr 5, 2023

fliespl commented Sep 27, 2023

lforst commented Sep 28, 2023 •

edited

fliespl commented Sep 28, 2023

lforst commented Sep 29, 2023 •

edited

fliespl commented Sep 29, 2023

feat(browser): Add captureUserFeedback #7729

feat(browser): Add captureUserFeedback #7729

Conversation

krystofwoldrich commented Apr 4, 2023 • edited

Next

github-actions bot commented Apr 4, 2023 • edited

size-limit report 📦

AbhiPrasad Apr 5, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fliespl commented Sep 27, 2023

lforst commented Sep 28, 2023 • edited

fliespl commented Sep 28, 2023

lforst commented Sep 29, 2023 • edited

fliespl commented Sep 29, 2023

feat(browser): Add `captureUserFeedback` #7729

feat(browser): Add `captureUserFeedback` #7729

krystofwoldrich commented Apr 4, 2023 •

edited

github-actions bot commented Apr 4, 2023 •

edited

AbhiPrasad Apr 5, 2023 •

edited

lforst commented Sep 28, 2023 •

edited

lforst commented Sep 29, 2023 •

edited