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

feat(vue): Add Pinia plugin #13841

Merged
merged 10 commits into from
Oct 15, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
},
"dependencies": {
"@sentry/vue": "latest || *",
"pinia": "^2.2.3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add pinia as a peer dependency as well with major v2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, added pinia as an optional peer dependency. 👍

"vue": "^3.4.15",
"vue-router": "^4.2.5"
},
Expand Down
14 changes: 14 additions & 0 deletions dev-packages/e2e-tests/test-applications/vue-3/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import { createApp } from 'vue';
import App from './App.vue';
import router from './router';

import { createPinia } from 'pinia';

import * as Sentry from '@sentry/vue';
import { browserTracingIntegration } from '@sentry/vue';

const app = createApp(App);
const pinia = createPinia();

Sentry.init({
app,
Expand All @@ -22,5 +25,16 @@ Sentry.init({
trackComponents: ['ComponentMainView', '<ComponentOneView>'],
});

pinia.use(
Sentry.createSentryPiniaPlugin({
actionTransformer: action => `Transformed: ${action}`,
stateTransformer: state => ({
transformed: true,
...state,
}),
}),
);

app.use(pinia);
app.use(router);
app.mount('#app');
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ const router = createRouter({
path: '/components',
component: () => import('../views/ComponentMainView.vue'),
},
{
path: '/cart',
component: () => import('../views/CartView.vue'),
},
],
});

Expand Down
43 changes: 43 additions & 0 deletions dev-packages/e2e-tests/test-applications/vue-3/src/stores/cart.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { acceptHMRUpdate, defineStore } from 'pinia';

export const useCartStore = defineStore({
id: 'cart',
state: () => ({
rawItems: [] as string[],
}),
getters: {
items: (state): Array<{ name: string; amount: number }> =>
state.rawItems.reduce(
(items, item) => {
const existingItem = items.find(it => it.name === item);

if (!existingItem) {
items.push({ name: item, amount: 1 });
} else {
existingItem.amount++;
}

return items;
},
[] as Array<{ name: string; amount: number }>,
),
},
actions: {
addItem(name: string) {
this.rawItems.push(name);
},

removeItem(name: string) {
const i = this.rawItems.lastIndexOf(name);
if (i > -1) this.rawItems.splice(i, 1);
},

throwError() {
throw new Error('error');
},
},
});

if (import.meta.hot) {
import.meta.hot.accept(acceptHMRUpdate(useCartStore, import.meta.hot));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<template>
<Layout>
<div>
<div style="margin: 1rem 0;">
<PiniaLogo />
</div>

<form @submit.prevent="addItemToCart" data-testid="add-items">
<input id="item-input" type="text" v-model="itemName" />
<button id="item-add">Add</button>
<button id="throw-error" @click="throwError">Throw error</button>
</form>

<form>
<ul data-testid="items">
<li v-for="item in cart.items" :key="item.name">
{{ item.name }} ({{ item.amount }})
<button
@click="cart.removeItem(item.name)"
type="button"
>X</button>
</li>
</ul>

<button
:disabled="!cart.items.length"
@click="clearCart"
type="button"
data-testid="clear"
>Clear the cart</button>
</form>
</div>
</Layout>
</template>

<script lang="ts">
import { defineComponent, ref } from 'vue'
import { useCartStore } from '../stores/cart'


export default defineComponent({
setup() {
const cart = useCartStore()

const itemName = ref('')

function addItemToCart() {
if (!itemName.value) return
cart.addItem(itemName.value)
itemName.value = ''
}

function throwError() {
throw new Error('This is an error')
}

function clearCart() {
if (window.confirm('Are you sure you want to clear the cart?')) {
cart.rawItems = []
}
}

// @ts-ignore
window.stores = { cart }

return {
itemName,
addItemToCart,
cart,

throwError,
clearCart,
}
},
})
</script>

<style scoped>
img {
width: 200px;
}

button,
input {
margin-right: 0.5rem;
margin-bottom: 0.5rem;
}
</style>
35 changes: 35 additions & 0 deletions dev-packages/e2e-tests/test-applications/vue-3/tests/pinia.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { expect, test } from '@playwright/test';
import { waitForError } from '@sentry-internal/test-utils';

test('sends pinia action breadcrumbs and state context', async ({ page }) => {
await page.goto('/cart');

await page.locator('#item-input').fill('item');
await page.locator('#item-add').click();

const errorPromise = waitForError('vue-3', async errorEvent => {
return errorEvent?.exception?.values?.[0].value === 'This is an error';
});

await page.locator('#throw-error').click();

const error = await errorPromise;

expect(error).toBeTruthy();
expect(error.breadcrumbs?.length).toBeGreaterThan(0);

const actionBreadcrumb = error.breadcrumbs?.find(breadcrumb => breadcrumb.category === 'action');

expect(actionBreadcrumb).toBeDefined();
expect(actionBreadcrumb?.message).toBe('Transformed: addItem');
expect(actionBreadcrumb?.level).toBe('info');

const stateContext = error.contexts?.state?.state;

expect(stateContext).toBeDefined();
expect(stateContext?.type).toBe('pinia');
expect(stateContext?.value).toEqual({
transformed: true,
rawItems: ['item'],
});
});
8 changes: 7 additions & 1 deletion packages/vue/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@
"@sentry/utils": "8.34.0"
},
"peerDependencies": {
"vue": "2.x || 3.x"
"vue": "2.x || 3.x",
"pinia": "2.x"
},
"peerDependenciesMeta": {
"pinia": {
"optional": true
}
},
"devDependencies": {
"vue": "~3.2.41"
Expand Down
1 change: 1 addition & 0 deletions packages/vue/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export { browserTracingIntegration } from './browserTracingIntegration';
export { attachErrorHandler } from './errorhandler';
export { createTracingMixins } from './tracing';
export { vueIntegration } from './integration';
export { createSentryPiniaPlugin } from './pinia';
103 changes: 103 additions & 0 deletions packages/vue/src/pinia.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { addBreadcrumb, getClient, getCurrentScope, getGlobalScope } from '@sentry/core';
import { addNonEnumerableProperty } from '@sentry/utils';

// Inline PiniaPlugin type
type PiniaPlugin = (context: {
store: {
$id: string;
$state: unknown;
$onAction: (callback: (context: { name: string; after: (callback: () => void) => void }) => void) => void;
};
}) => void;

type SentryPiniaPluginOptions = {
attachPiniaState?: boolean;
addBreadcrumbs?: boolean;
actionTransformer?: (action: any) => any;
stateTransformer?: (state: any) => any;
};

export const createSentryPiniaPlugin: (options?: SentryPiniaPluginOptions) => PiniaPlugin = (
options: SentryPiniaPluginOptions = {
attachPiniaState: true,
addBreadcrumbs: true,
actionTransformer: action => action,
stateTransformer: state => state,
},
) => {
const plugin: PiniaPlugin = ({ store }) => {
options.attachPiniaState !== false &&
getGlobalScope().addEventProcessor((event, hint) => {
try {
// Get current timestamp in hh:mm:ss
const timestamp = new Date().toTimeString().split(' ')[0];
const filename = `pinia_state_${store.$id}_${timestamp}.json`;

hint.attachments = [
...(hint.attachments || []),
{
filename,
data: JSON.stringify(store.$state),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M: What just came to my mind: the store can contain PII, especially when pinia is used in forms. What do you think of adding the type of the value as the value? Like this:

name: string
likes: object

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the pinia plugin needs to be added manually I would argue it is fine. Just having the type is basically useless for debugging.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stringifying the state as JSON will only work in some scenarios and fail in many others like Dates, Errors, Maps, Sets, and custom classes. I would recommend using https://github.com/Rich-Harris/devalue like Nuxt does to allow users to provide serializer and revivers. I would even recommend adding a custom one that uses a toJSON() method if it exists by default as a convention. I think, but I'm not sure, that Nuxt has something similar.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, we are on a tight bundle-size budget so we need to be a bit careful what we pull in as deps.

What we definitely should do is wrap the stringify call in a try-catch because it can throw in certain situations.

Non-serializable data is tricky and I would recommend we do it in a follow-up if at all. Dates get serialized properly with ordinary JSON.stringify, errors, maps, and sets, will become objects (which is fine for now), and stringify already supports toJSON natively.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The toJSON() was specific to devalue ofc. If users can opt-in to a fully compatible serializable solution, they would be able to put their app in a similar state with the serialized state and debug in dev mode, so I think it's nice to advocate for that! Since Map and Sets are common, it would be a shame to lose them fully by using JSON.stringify()`

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this event processor logic is already in try-catch, so we're safe on that part. We can do a follow-up for serializing. Thanks for the review @posva!

},
];
} catch (_) {
// empty
}

return event;
});

store.$onAction(context => {
context.after(() => {
const transformedActionName = options.actionTransformer
? options.actionTransformer(context.name)
: context.name;

if (
typeof transformedActionName !== 'undefined' &&
transformedActionName !== null &&
options.addBreadcrumbs !== false
) {
addBreadcrumb({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add an option to turn off creating breadcrumbs. From experience, some apps have a lot of state updates and breadcrumbs might be very spammy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated: ea7ba22

category: 'action',
message: transformedActionName,
level: 'info',
});
}

/* Set latest state to scope */
const transformedState = options.stateTransformer ? options.stateTransformer(store.$state) : store.$state;
const scope = getCurrentScope();
const currentState = scope.getScopeData().contexts.state;

if (typeof transformedState !== 'undefined' && transformedState !== null) {
const client = getClient();
const options = client && client.getOptions();
const normalizationDepth = (options && options.normalizeDepth) || 3; // default state normalization depth to 3
const piniaStateContext = { type: 'pinia', value: transformedState };

const newState = {
...(currentState || {}),
state: piniaStateContext,
};

addNonEnumerableProperty(
newState,
'__sentry_override_normalization_depth__',
3 + // 3 layers for `state.value.transformedState
normalizationDepth, // rest for the actual state
);

scope.setContext('state', newState);
} else {
scope.setContext('state', {
...(currentState || {}),
state: { type: 'pinia', value: 'undefined' },
});
}
});
});
};

return plugin;
};
Loading
Loading