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(server): Add Reporting API server helpers #14044

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Oct 21, 2024

The Sentry backend doesn't support the Reporting API yet but since same-origin reports have cookies/authentication included, it will often make more sense to send these via your server so you can include user context via middleware etc.

This PR:

  • Adds the required envelope types
  • Adds utilities to create raw CSP security envelopes
  • Adds utility to convert Reporting API CSP report format to the older one the Sentry backend uses
  • Add and export new function from @sentry/core that help capture reports (below)
  • Handles crash reports as new events

New @sentry/core exports:

/** Captures Reports from the Reporting API */
function captureReportingApi(reports: Report[], options: {client?: Client}): Promise<void> {}

Express example:

import { captureReportingApi } from '@sentry/core';
import * as Sentry from '@sentry/node';
import express from 'express';

Sentry.init({
  dsn: 'https://[email protected]/1337',
});

const app = express();

app.post('/reporting-api', express.json({ type: 'application/reports+json' }), async (req, res) => {
  await captureReportingApi(req.body);
  res.sendStatus(200);
});

Copy link
Contributor

github-actions bot commented Oct 21, 2024

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.74 KB +0.07% +14 B 🔺
@sentry/browser - with treeshaking flags 21.53 KB +0.06% +13 B 🔺
@sentry/browser (incl. Tracing) 35.13 KB +0.04% +13 B 🔺
@sentry/browser (incl. Tracing, Replay) 71.84 KB +0.03% +19 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.27 KB +0.03% +14 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 76.15 KB +0.03% +20 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 88.95 KB +0.02% +14 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback, metrics) 90.79 KB +0.02% +13 B 🔺
@sentry/browser (incl. metrics) 26.99 KB +0.06% +15 B 🔺
@sentry/browser (incl. Feedback) 39.88 KB +0.04% +13 B 🔺
@sentry/browser (incl. sendFeedback) 27.39 KB +0.05% +12 B 🔺
@sentry/browser (incl. FeedbackAsync) 32.18 KB +0.05% +16 B 🔺
@sentry/react 25.5 KB +0.05% +13 B 🔺
@sentry/react (incl. Tracing) 38.08 KB +0.03% +10 B 🔺
@sentry/vue 26.89 KB +0.06% +14 B 🔺
@sentry/vue (incl. Tracing) 37 KB +0.04% +14 B 🔺
@sentry/svelte 22.88 KB +0.06% +14 B 🔺
CDN Bundle 24.1 KB +0.06% +13 B 🔺
CDN Bundle (incl. Tracing) 36.94 KB +0.05% +17 B 🔺
CDN Bundle (incl. Tracing, Replay) 71.6 KB +0.02% +14 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 76.94 KB +0.03% +16 B 🔺
CDN Bundle - uncompressed 70.64 KB +0.04% +24 B 🔺
CDN Bundle (incl. Tracing) - uncompressed 109.63 KB +0.03% +24 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 222.15 KB +0.02% +24 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 235.37 KB +0.01% +24 B 🔺
@sentry/nextjs (client) 38.16 KB +0.04% +13 B 🔺
@sentry/sveltekit (client) 35.72 KB +0.03% +10 B 🔺
@sentry/node 129.61 KB +0.02% +14 B 🔺
@sentry/node - without tracing 94.32 KB +0.02% +15 B 🔺
@sentry/aws-serverless 105.18 KB +0.01% +9 B 🔺

View base workflow run

@timfish timfish marked this pull request as ready for review October 29, 2024 12:56
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

I'm a little hesitant in the public API requiring browserStackParser, but I guess there's no good way around this.

I also think handleReportingApi should be captureX instead. What do you think?

@timfish
Copy link
Collaborator Author

timfish commented Oct 29, 2024

I'm a little hesitant in the public API requiring browserStackParser, but I guess there's no good way around this.

We should pull stack parsing for now since:

  • Origin trials should not be used with all users in production
  • It might never make it to production Chrome
  • I haven't managed to get it working

I also think handleReportingApi should be captureX instead. What do you think?

Yep, much better!

@samuelmaddock
Copy link

samuelmaddock commented Nov 11, 2024

We should pull stack parsing for now since:

  • I haven't managed to get it working

I've managed to get this working in an Electron application. Here's what was required:

  • Add document-policy: include-js-call-stacks-in-crash-reports response header to page
  • Launch Chromium with --enable-features=DocumentPolicyIncludeJSCallStacksInCrashReports flag

From reading the browser and renderer process code, I believe this should be enough to send the call stacks. So far this data has been valuable for us so I'd love to see it supported in Sentry!


One more thing to note. DevTools needs to be closed for the renderer to become unresponsive and send the call stack. I wrote this snippet to test.

function startInfiniteLoop () {
  console.log('spinning');
  while (1) {
    document.documentElement.getBoundingClientRect();
  }
}

// Click the document to start the infinite loop
document.documentElement.addEventListener('click', function clickHandler () {
  startInfiniteLoop();
});

@timfish
Copy link
Collaborator Author

timfish commented Nov 12, 2024

@samuelmaddock could Electron pass the stack trace through in the webContents unresponsive event?

I haven't looked at the code, I'm simply guessing it uses similar code paths to triggering the Reporting API unresponsive events

@samuelmaddock
Copy link

@timfish I started working on this in electron/electron#44204

I'm hoping to get back to that PR soon. The call stacks use a separate async request so Electron may need to delay unresponsive to wait for that data if that's the design we go with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants