Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat(server): Add Reporting API server helpers #14044

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

Open
timfish wants to merge 7 commits into develop
base: develop
Choose a base branch
Loading
from timfish/feat/reporting-api

Conversation

Copy link
Collaborator

@timfish timfish commented Oct 21, 2024
edited
Loading

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://public@dsn.ingest.sentry.io/1337',
});
const app = express();
app.post('/reporting-api', express.json({ type: 'application/reports+json' }), async (req, res) => {
 await captureReportingApi(req.body);
 res.sendStatus(200);
});

chrisweb, samuelmaddock, x0good, and petamoriken reacted with thumbs up emoji
Copy link
Contributor

github-actions bot commented Oct 21, 2024
edited
Loading

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?

Copy link
Collaborator Author

timfish commented Oct 29, 2024
edited
Loading

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!

Copy link

samuelmaddock commented Nov 11, 2024
edited
Loading

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 reacted with thumbs up emoji AbhiPrasad reacted with eyes emoji

Copy link
Collaborator Author

timfish commented Nov 12, 2024
edited
Loading

@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

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.

timfish reacted with heart emoji

Copy link

什么时候可以被合并?
When can it be merged?

Copy link
Member

@cloudcome this is not prioritzed at the moment given we have some other things to get done first. We'll push this forward when this becomes a priority again. Thanks for your patience in the mean time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AbhiPrasad AbhiPrasad AbhiPrasad left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /