-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(browser): Add captureUserFeedback
#7729
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
Changes from 6 commits
1407448
314b615
a0394a0
e295944
6b8b10c
f29a85f
2c5aae5
631d858
d30527c
31c54c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import * as Sentry from '@sentry/browser'; | ||
|
||
window.Sentry = Sentry; | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
Sentry.captureUserFeedback({ | ||
eventId: 'test_event_id', | ||
email: 'test_email', | ||
comments: 'test_comments', | ||
name: 'test_name', | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import { expect } from '@playwright/test'; | ||
import type { UserFeedback } from '@sentry/types'; | ||
|
||
import { sentryTest } from '../../../../utils/fixtures'; | ||
import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; | ||
|
||
sentryTest('should capture simple user feedback', async ({ getLocalTestPath, page }) => { | ||
const url = await getLocalTestPath({ testDir: __dirname }); | ||
|
||
const eventData = await getFirstSentryEnvelopeRequest<UserFeedback>(page, url); | ||
|
||
expect(eventData).toMatchObject({ | ||
eventId: 'test_event_id', | ||
email: 'test_email', | ||
comments: 'test_comments', | ||
name: 'test_name', | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
import { getCurrentHub } from '@sentry/core'; | ||
import type { DsnComponents, EventEnvelope, SdkMetadata, UserFeedback, UserFeedbackItem } from '@sentry/types'; | ||
import { createEnvelope, dsnToString, logger } from '@sentry/utils'; | ||
|
||
/** | ||
* Sends user feedback to Sentry. | ||
*/ | ||
export function captureUserFeedback(feedback: UserFeedback): void { | ||
const hub = getCurrentHub(); | ||
const client = hub.getClient(); | ||
const transport = client && client.getTransport(); | ||
|
||
if (!client) { | ||
__DEBUG_BUILD__ && logger.log('[UserFeedback] getClient did not return a Client, user feedback will not be sent.'); | ||
return; | ||
} | ||
|
||
if (!transport) { | ||
__DEBUG_BUILD__ && | ||
logger.log('[UserFeedback] getTransport did not return a Transport, user feedback will not be sent.'); | ||
krystofwoldrich marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return; | ||
} | ||
|
||
const envelope = createUserFeedbackEnvelope(feedback, { | ||
metadata: client.getSdkMetadata && client.getSdkMetadata(), | ||
dsn: client.getDsn(), | ||
tunnel: client.getOptions().tunnel, | ||
}); | ||
|
||
void transport.send(envelope); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's times like this I wish m: We can only call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I have a feeling this should be checked in a more central location. Doesn't have to be this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Probably? It seems safer that this whole chain of grabbing stuff off the hub that may or may not be there. But honestly 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me @krystofwoldrich - sorry for making you revert your work, but let's just add the methods to the browser client. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, that's fair not that many SDKs, just a note RN doesn't wrap Browser. |
||
} | ||
|
||
/** | ||
* Creates an envelope from a user feedback. | ||
*/ | ||
export function createUserFeedbackEnvelope( | ||
feedback: UserFeedback, | ||
{ | ||
metadata, | ||
tunnel, | ||
dsn, | ||
}: { | ||
metadata: SdkMetadata | undefined; | ||
tunnel: string | undefined; | ||
dsn: DsnComponents | undefined; | ||
}, | ||
): EventEnvelope { | ||
const headers: EventEnvelope[0] = { | ||
event_id: feedback.event_id, | ||
sent_at: new Date().toISOString(), | ||
...(metadata && | ||
metadata.sdk && { | ||
sdk: { | ||
name: metadata.sdk.name, | ||
version: metadata.sdk.version, | ||
}, | ||
}), | ||
...(!!tunnel && !!dsn && { dsn: dsnToString(dsn) }), | ||
}; | ||
const item = createUserFeedbackEnvelopeItem(feedback); | ||
|
||
return createEnvelope(headers, [item]); | ||
} | ||
|
||
function createUserFeedbackEnvelopeItem(feedback: UserFeedback): UserFeedbackItem { | ||
const feedbackHeaders: UserFeedbackItem[0] = { | ||
type: 'user_report', | ||
}; | ||
return [feedbackHeaders, feedback]; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import { createUserFeedbackEnvelope } from '../../src/userfeedback'; | ||
|
||
describe('userFeedback', () => { | ||
test('creates user feedback envelope header', () => { | ||
const envelope = createUserFeedbackEnvelope( | ||
{ | ||
comments: 'Test Comments', | ||
email: '[email protected]', | ||
name: 'Test User', | ||
event_id: 'testEvent123', | ||
}, | ||
{ | ||
metadata: { | ||
sdk: { | ||
name: 'testSdkName', | ||
version: 'testSdkVersion', | ||
}, | ||
}, | ||
tunnel: 'testTunnel', | ||
dsn: { | ||
host: 'testHost', | ||
projectId: 'testProjectId', | ||
protocol: 'http', | ||
}, | ||
}, | ||
); | ||
|
||
expect(envelope[0]).toEqual({ | ||
dsn: 'http://undefined@testHost/undefinedtestProjectId', | ||
event_id: 'testEvent123', | ||
sdk: { | ||
name: 'testSdkName', | ||
version: 'testSdkVersion', | ||
}, | ||
sent_at: expect.any(String), | ||
}); | ||
}); | ||
|
||
test('creates user feedback envelope item', () => { | ||
const envelope = createUserFeedbackEnvelope( | ||
{ | ||
comments: 'Test Comments', | ||
email: '[email protected]', | ||
name: 'Test User', | ||
event_id: 'testEvent123', | ||
}, | ||
{ | ||
metadata: undefined, | ||
tunnel: undefined, | ||
dsn: undefined, | ||
}, | ||
); | ||
|
||
expect(envelope[1]).toEqual([ | ||
[ | ||
{ | ||
type: 'user_report', | ||
}, | ||
{ | ||
comments: 'Test Comments', | ||
email: '[email protected]', | ||
name: 'Test User', | ||
event_id: 'testEvent123', | ||
}, | ||
], | ||
]); | ||
}); | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.