-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(vue): Apply stateTransformer to attachments in createSentryPiniaPlugin #14474
base: develop
Are you sure you want to change the base?
Conversation
Thanks for the PR @sircharlo. Implementation looks good to me, could you also add a test for this? |
Erm I can try. Not sure, I had started adding something in e2e but in the Discord I was told that was the wrong place for it. |
As far as I see we only have pinia tests in the vue-3 and nuxt-4 E2E test apps, so adding tests to one of these (probably vue-3?) sounds reasonable to me! Not sure if others (cc @onurtemizkan or @s1gr1d ) have other ideas for this. |
@mydea, yes I also think that's reasonable (and easier) to add cases to e2e tests. IMHO a new case here will be enough for this PR. @sircharlo, please let me know if you have any issues running the E2E tests. |
@onurtemizkan Thanks for the feedback! Ok, I'll continue where I had left off in that file then, as that's where I had started writing a test. Would this be the correct way to access an attachment from the current scope? I couldn't find an easy way to access the global scope from the const scopeData = error?.sdkProcessingMetadata?.capturedSpanScope?.getScopeData();
const stateAttachment = scopeData?.attachments?.find(attachment => attachment.filename.includes('pinia_state_')); |
You can wait for an attachment envelope item like this: function waitForAttachment(
proxyServerName: string,
callback: (attachment: AttachmentItem[1]) => Promise<boolean> | boolean,
): Promise<AttachmentItem[1]> {
const timestamp = getNanosecondTimestamp();
return new Promise((resolve, reject) => {
waitForEnvelopeItem(
proxyServerName,
async envelopeItem => {
if (envelopeItem[0].type === 'attachment') {
const attachmentItem = envelopeItem[1] as AttachmentItem[1];
if (await callback(attachmentItem)) {
resolve(attachmentItem);
return true;
}
}
return false;
},
timestamp,
).catch(reject);
});
} And then use it like this: const errorPromise = waitForError('vue-3', async errorEvent => {
return errorEvent?.exception?.values?.[0].value === 'This is an error';
});
const attachmentPromise = waitForAttachment('vue-3', async attachment => {
return attachment = '...';
});
await page.locator('#throw-error').click();
const error = await errorPromise;
const attachment = await attachmentPromise;
expect(attachment).toEqual(...); |
This PR modifies the
createSentryPiniaPlugin
to apply thestateTransformer
to attachments, in addition to the context.Why?
Attachments can quickly become large when the full
store.$state
is included, especially if there are items that do not need to be reported to Sentry. By applying thestateTransformer
, developers can reduce the size of attachments, avoid unnecessary data uploads, and stay within their Sentry usage limits.Changes
store.$state
using thestateTransformer
(if provided) before including it in the attachments as well as the context.Benefits
Let me know if further adjustments are needed!
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint
) & (yarn test
).