-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(node): Add Hapi Integration #9539
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
Conversation
0b5d80c
to
ddc49b1
Compare
@@ -0,0 +1,163 @@ | |||
import type { Boom } from '@hapi/boom'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not/cannot use types like these, as then we need to add hapi as a dependency 😬 let's just inline the types we need in here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍 21beab3
const sentryRequest = https.request( | ||
sentryIngestUrl, | ||
{ headers: proxyRequest.headers, method: proxyRequest.method }, | ||
sentryResponse => { | ||
sentryResponse.addListener('data', (chunk: Buffer) => { | ||
proxyResponse.write(chunk, 'binary'); | ||
sentryResponseChunks.push(chunk); | ||
}); | ||
|
||
sentryResponse.addListener('end', () => { | ||
eventCallbackListeners.forEach(listener => { | ||
const rawSentryResponseBody = Buffer.concat(sentryResponseChunks).toString(); | ||
|
||
const data: SentryRequestCallbackData = { | ||
envelope: parseEnvelope(proxyRequestBody, new TextEncoder(), new TextDecoder()), | ||
rawProxyRequestBody: proxyRequestBody, | ||
rawSentryResponseBody, | ||
sentryResponseStatusCode: sentryResponse.statusCode, | ||
}; | ||
|
||
listener(Buffer.from(JSON.stringify(data)).toString('base64')); | ||
}); | ||
proxyResponse.end(); | ||
}); | ||
|
||
sentryResponse.addListener('error', err => { | ||
throw err; | ||
}); | ||
|
||
proxyResponse.writeHead(sentryResponse.statusCode || 500, sentryResponse.headers); | ||
}, | ||
); |
Check failure
Code scanning / CodeQL
Server-side request forgery
size-limit report 📦
|
0729c7f
to
c5c9d9c
Compare
} | ||
|
||
function sendErrorToSentry(errorData: object): void { | ||
captureException(errorData, scope => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: if we merge #9590 first, update this later!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, merged this in yet, so we can refactor this to this instead:
captureException(errorData, {
mechanism: {
type: 'hapi',
handled: false,
data: {
function: 'hapiErrorPlugin',
},
}
});
|
||
export const hapiErrorPlugin = { | ||
name: 'SentryHapiErrorPlugin', | ||
version: '0.0.1', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make this SDK_VERSION
, which we can import from core?
/* eslint-disable @typescript-eslint/no-empty-interface */ | ||
/* eslint-disable @typescript-eslint/no-namespace */ | ||
|
||
// Vendored and simplified from: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we possibly add the versions here we vendored from, may make future updating easier? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some small comments, but overall this looks great! nice E2E tests 🎉
Thanks for the review @mydea. I updated the PR 👍 |
3561347
to
d8bb0e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
back from my vacation, this looks great, thank you!
Resolves: #9344
Adds a new node integration for Hapi framework.
Also exports a Hapi plugin to capture errors when the tracing instrumentation from
node-experimental
is used.Can be used with
node-experimental
(Sample Error Event) like:Also can be used from
@sentry/node
with tracing (Errored Transaction, Successful Transaction) and error tracking (Event) like: