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: Add option to use own NSURLSession for transport #3811

Merged
merged 84 commits into from
May 7, 2024

Conversation

dKasabwala
Copy link
Contributor

@dKasabwala dKasabwala commented Apr 1, 2024

…on configuration.
#skip-changelog

📜 Description

  • We are a current customer of sentry, and we use custom socks proxy on our end, we have no way to set up socks proxy for sentry services. we have added change to provide hook with URL Session.

💡 Support Socks proxy and private session configuration with custom SSL Certs.

  • Solve Custom SSL pinning
  • Provide support Socks Proxy

💚 How did you test it?

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

This change is pretty simple and gives the user more power, which in my opinion make sense in here.

SentryOptions has a - (BOOL)validateOptions:(NSDictionary<NSString *, id> *)options didFailWithError:(NSError *_Nullable *_Nullable)error method that need to be updated to accommodate the new property.

We also need some tests to make sure the new property its being used properly.

Sources/Sentry/SentryTransportFactory.m Outdated Show resolved Hide resolved
Sources/Sentry/Public/SentryOptions.h Outdated Show resolved Hide resolved
@dKasabwala
Copy link
Contributor Author

  • Added unit tests around new options for urlSession
  • updated - (BOOL)validateOptions:(NSDictionary<NSString *, id> *)options didFailWithError:(NSError *_Nullable *_Nullable)error
  • new property description been added as well.

@kahest kahest linked an issue Apr 3, 2024 that may be closed by this pull request
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

I pushed some updates for code docs for the options to this branch. Please add some tests @dKasabwala, as pointed out, and an entry to the Changelog.

Thanks for doing this 😃.

Sources/Sentry/SentryTransportFactory.m Show resolved Hide resolved
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.743%. Comparing base (523915a) to head (4a7084e).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3811       +/-   ##
=============================================
- Coverage   90.761%   90.743%   -0.019%     
=============================================
  Files          586       586               
  Lines        45754     45783       +29     
  Branches     16335     16333        -2     
=============================================
+ Hits         41527     41545       +18     
- Misses        4047      4168      +121     
+ Partials       180        70      -110     
Files Coverage Δ
Sources/Sentry/SentryOptions.m 99.195% <100.000%> (+0.004%) ⬆️
Sources/Sentry/SentryTransportFactory.m 100.000% <100.000%> (ø)
...Tests/Networking/SentryTransportFactoryTests.swift 100.000% <100.000%> (ø)
Tests/SentryTests/SentryOptionsTest.m 98.619% <100.000%> (+0.012%) ⬆️

... and 30 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 523915a...4a7084e. Read the comment docs.

@dKasabwala
Copy link
Contributor Author

@philipphofmann Please get this approved, All checks are complete, we do need these changes next release version.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

The unit tests in CI are failing because the unit test don't compile @dKasabwala. You can ignore the other failing CI jobs. They fail due to our CI setup.

dependabot bot and others added 15 commits May 6, 2024 10:28
…ry#3810)

Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3.1.5 to 4.1.1.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](codecov/codecov-action@4fe8c5f...c16abc2)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Fix NSInvalidArgumentException for NSError sentryErrorWithDomain by
removing the category functions because they get stripped for static
libraries. This is related to getsentryGH-3764.

Fixes getsentryGH-3818
Again remove category functions to avoid NSInvalidArgumentException
when including Sentry statically.
Add an option to exclude certain classes from swizzling.

Fixes getsentryGH-3798
Fix a crash in SentryTracer hasUnfinishedChildSpansToWaitFor by not
setting the shouldIgnoreWaitForChildrenCallback to nil when finishing
the tracer.

Fixes getsentryGH-3781
tl;dr; Fixed SentrySwiftUI Carthage build import.

Since there is no modulemap for the SentrySwiftUI declaring exported submodules:

For Carthage build make @_implementationOnly import SentryInternal.
Import SentrySwiftUI xcframework Samples/Carthage-Validation/XCFramework project to check imports.

Co-authored-by: Dhiogo Brustolin <dhiogo.brustolin@sentry.io>
Add the timing API to measure the duration of a closure.
philipphofmann and others added 19 commits May 6, 2024 10:28
Instead of capturing and serializing the transaction on the calling
thread, the SDK now does this on a background thread.

Fixes getsentryGH-3826
If there is no UIWindow available during SDK initialization, wait for a UISceneDidActivateNotification
)

Bumps [dorny/paths-filter](https://github.com/dorny/paths-filter) from 3.0.0 to 3.0.2.
- [Release notes](https://github.com/dorny/paths-filter/releases)
- [Changelog](https://github.com/dorny/paths-filter/blob/master/CHANGELOG.md)
- [Commits](dorny/paths-filter@0bc4621...de90cc6)

---
updated-dependencies:
- dependency-name: dorny/paths-filter
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
The release name passed to the app state from the options is nullable.
The SentryAppState including the SentryWatchDogTerminationLogic
couldn't handle this edge case correctly, which is fixed now.
The ThreadSanitizer reports data races for the SentryFramesTracker,
which we can ignore.

Fixes getsentryGH-3702
Ignore the thread sanitizer in the code where possible, which has the
advantage of knowing a method is ignored when reading the code and not
jumping to the ThreadSanitizer.sup file. Furthermore, use the
ThreadSanitizer.sup file not only for tests but for the Run Xcode
schemes.
Every log message needs to acquire a lock to evaluate whether the logger
should log it. getsentry#3303 added this logic to fix a data race the thread
sanitizer job found. As the SDK only calls the configure method when
starting, we don't need to put a synchronized keyword around the code
that evaluates the log level. This PR replaces the synchronized keyword
by ignoring the thread sanitizer.
Move XCFrameworks zip files to the repository root folder after building
so that you can compile the Carthage sample locally.
Use the text color or the average color of the image as the redaction mask.

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
@dKasabwala
Copy link
Contributor Author

Fix up compiler error

@philipphofmann philipphofmann self-assigned this May 7, 2024
@philipphofmann philipphofmann changed the title [Updated] added Change to support socks proxy or private session configuration. feat: Add option to use own NSURLSession for transport May 7, 2024
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM

@philipphofmann philipphofmann merged commit 99ab5d0 into getsentry:main May 7, 2024
63 of 68 checks passed
threema-matteo pushed a commit to threema-ch/sentry-cocoa that referenced this pull request May 21, 2024
Add an option to use their own NSURLSession for transports so users can configure a proxy and such.

Fixes getsentryGH-3052

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
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.

Add custom HTTPHeaders in URLRequest
9 participants