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(core): Add ServerRuntimeClient #8930

Merged
merged 6 commits into from
Sep 5, 2023

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Sep 1, 2023

ref: #8693

The ServerRuntimeClient is a near identical copy of the nextjs EdgeClient. To make it a direct replacement it has constructor options to override the event platform, runtime, and server_name.

This PR makes yet another copy of the Node eventbuilder.ts but after future PRs to remove the EdgeClient and make NodeClient extend ServerRuntimeClient, this will be the only copy. I've put the eventbuilder code in utils since some of these functions are used elsewhere outside of the clients and I don't want to export these from core and them become part of our public API. This is especially important since the browser SDK already exports it's own slightly different exceptionFromError.

@timfish timfish requested a review from lforst September 1, 2023 11:42
if (isPlainObject(exception)) {
// This will allow us to group events based on top-level keys
// which is much better than creating new group when any key/value change
const message = `Non-Error exception captured with keys: ${extractExceptionKeysForMessage(exception)}`;
Copy link
Member

Choose a reason for hiding this comment

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

l: I wonder if we want to align this with the changes we did for browser-based non-error-object capturing: #8374

But maybe we do that in a separate follow-up PR!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There has been some discussion around consolidating the node/browser eventbuilders too...

Copy link
Member

Choose a reason for hiding this comment

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

let's do this in a follow up PR after we merge in node sdk changes re: server runtime!

if (isPlainObject(exception)) {
// This will allow us to group events based on top-level keys
// which is much better than creating new group when any key/value change
const message = `Non-Error exception captured with keys: ${extractExceptionKeysForMessage(exception)}`;
Copy link
Member

Choose a reason for hiding this comment

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

let's do this in a follow up PR after we merge in node sdk changes re: server runtime!

@AbhiPrasad AbhiPrasad merged commit f54e121 into getsentry:develop Sep 5, 2023
68 checks passed
@timfish timfish deleted the feat/add-ServerRuntimeClient branch September 14, 2023 14:22
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