ref: Remove - [SentryOptions initWithDict:didFailWithError:]

[kevinrenskers]

📜 Description

Removed - [SentryOptions initWithDict:didFailWithError:]
Removed - [SentrySDK startWithOptions:]
Removed deprecated - [SentryOptions sdkInfo]

I did introduce a new - [SentryOptions initWithDsn:didFailWithError:] method to make it easy to create SentryOptions with a DSN (the most common usage of SentryOptions), while keeping the throwing behavior when the DSN is invalid.

Question 1: I think we should rename - [SentrySDK startWithOptionsObject:] to - [SentrySDK startWithOptions:]. Do you agree?

Question 2: I don't know if this is a feature or a fix? But I went with "ref" now.

Question 3: How do we add breaking changes like this to the changelog?

💡 Motivation and Context

Closes #2395

💚 How did you test it?

Unit tests

📝 Checklist

• I reviewed the submitted code
• I added tests to verify the changes
• I updated the docs if needed
• Review from the native team if needed
• No breaking changes -> oh yes there are!

🔮 Next steps

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1226.48 ms	1257.34 ms	30.86 ms
Size	20.75 KiB	373.94 KiB	353.19 KiB

Baseline results on branch: 8.0.0

Startup times

Revision	Plain	With Sentry	Diff
58ec104	1244.29 ms	1269.67 ms	25.39 ms
a9e77dc	1231.94 ms	1254.85 ms	22.91 ms
d10145a	1232.65 ms	1257.55 ms	24.90 ms
7eee302	1228.73 ms	1241.94 ms	13.21 ms
dcac8ad	1238.82 ms	1247.80 ms	8.98 ms

App size

Revision	Plain	With Sentry	Diff
58ec104	20.75 KiB	379.11 KiB	358.36 KiB
a9e77dc	20.75 KiB	379.12 KiB	358.36 KiB
d10145a	20.75 KiB	379.12 KiB	358.36 KiB
7eee302	20.75 KiB	374.73 KiB	353.97 KiB
dcac8ad	20.75 KiB	379.11 KiB	358.36 KiB

Previous results on branch: ref/2395-remove-options-dict

Startup times

Revision	Plain	With Sentry	Diff
bd0eaeb	1238.12 ms	1246.08 ms	7.96 ms
87e351e	1221.43 ms	1234.47 ms	13.04 ms
74e0fc3	1244.22 ms	1270.11 ms	25.88 ms
397b93b	1233.94 ms	1267.76 ms	33.82 ms
e09ff64	1234.02 ms	1240.94 ms	6.92 ms
b7bcebd	1217.47 ms	1240.48 ms	23.01 ms
82acb6a	1235.70 ms	1253.28 ms	17.58 ms
bef2c08	1222.90 ms	1226.51 ms	3.61 ms

App size

Revision	Plain	With Sentry	Diff
bd0eaeb	20.75 KiB	369.76 KiB	349.00 KiB
87e351e	20.75 KiB	369.87 KiB	349.12 KiB
74e0fc3	20.75 KiB	369.82 KiB	349.07 KiB
397b93b	20.75 KiB	369.87 KiB	349.12 KiB
e09ff64	20.75 KiB	369.87 KiB	349.12 KiB
b7bcebd	20.75 KiB	373.94 KiB	353.19 KiB
82acb6a	20.75 KiB	369.87 KiB	349.12 KiB
bef2c08	20.75 KiB	373.94 KiB	353.18 KiB

[kevinrenskers]

Note: we should change the base branch to some sort of 8.0.0 branch, before merging.

[kevinrenskers]

When I run - [SentryTests testVersion] locally by itself, or by running all the SentryTests.m tests, it always succeeds. But when I run the entire test suite, it always fails, also here on CI. I am not sure what is going on.

testVersion: (([version isEqualToString:SentryMeta.versionString]) is true) failed - Version of bundle:7.31.1 not equal to version of SentryMeta:0.0.1

What is this test even trying to test? 🤔

edit: turns out I had the exact same problem and question in #1960. I hate this test, how it's so easily broken by global variables.

[brustolin]

Question 1: I think we should rename - [SentrySDK startWithOptionsObject:] to - [SentrySDK startWithOptions:]. Do you agree?

Yes, sounds good to me.

[philipphofmann]

I did introduce a new - [SentryOptions initWithDsn:didFailWithError:] method to make it easy to create SentryOptions with a DSN (the most common usage of SentryOptions), while keeping the throwing behavior when the DSN is invalid.

I would vote for removing this one.

Question 1: I think we should rename - [SentrySDK startWithOptionsObject:] to - [SentrySDK startWithOptions:]. Do you agree?

Yes.

Question 2: I don't know if this is a feature or a fix? But I went with "ref" now.

Ref is fine.

Question 3: How do we add breaking changes like this to the changelog?

I think that's already answered.

[kevinrenskers]

I did introduce a new - [SentryOptions initWithDsn:didFailWithError:] method to make it easy to create SentryOptions with a DSN (the most common usage of SentryOptions), while keeping the throwing behavior when the DSN is invalid.

I would vote for removing this one.

Basically every time you instantiate a SentryOptions object, you do that with a DSN. Removing this convenience initializer makes that harder to do to, less obvious you need to set a DSN, and last but not least: you get rid of the throwing behavior when the DSN is invalid. Instead the dev will only get console logs.

Why would you want to remove this?

[philipphofmann]

Why would you want to remove this?

Ups sorry. My bad. It was late. [SentryOptions initWithDsn:didFailWithError:] I thought it was [SentrySDK initWithDsn:didFailWithError:]. Please keep it.

[philipphofmann]

LGTM

[github-actions]

	Fails
🚫	Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Remove `- [SentryOptions initWithDict:didFailWithError:]` ([#2404](https://github.com/getsentry/sentry-cocoa/pull/2404))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 0bf85b3