Skip to content

fix(apple): don't panic in FFI functions#8202

Merged
thomaseizinger merged 7 commits into
mainfrom
fix/apple-no-panic
Feb 20, 2025
Merged

fix(apple): don't panic in FFI functions#8202
thomaseizinger merged 7 commits into
mainfrom
fix/apple-no-panic

Conversation

@thomaseizinger

Copy link
Copy Markdown
Member

Now that we have error reporting via Sentry in Swift-land as well, we can handle errors in the FFI layer more gracefully and return them to Swift.

@vercel

vercel Bot commented Feb 19, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 19, 2025 11:21pm

@sentry

sentry Bot commented Feb 19, 2025

Copy link
Copy Markdown

Sentry Issue: APPLE-CLIENT-2N

@sentry

sentry Bot commented Feb 19, 2025

Copy link
Copy Markdown

Sentry Issue: APPLE-CLIENT-2P

session?.setDns(jsonResolvers)
do {
session?.setDns(jsonResolvers)
} catch let error {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
} catch let error {
} catch {

error is already provided

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Where? I just copied this from another do catch block. Is error implicitly defined within a catch?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, it is - that one must have been leftover from Earlier Times™

session?.setDisabledResources(toSet)
do {
session?.setDisabledResources(toSet)
} catch let error {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
} catch let error {
} catch {

Comment thread swift/apple/FirezoneNetworkExtension/Adapter.swift Outdated
Comment thread website/src/components/Changelog/Apple.tsx Outdated
@thomaseizinger
thomaseizinger added this pull request to the merge queue Feb 19, 2025
@jamilbk
jamilbk removed this pull request from the merge queue due to a manual request Feb 19, 2025
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
@jamilbk
jamilbk enabled auto-merge February 19, 2025 23:19
@jamilbk
jamilbk added this pull request to the merge queue Feb 19, 2025
github-merge-queue Bot pushed a commit that referenced this pull request Feb 19, 2025
Now that we have error reporting via Sentry in Swift-land as well, we
can handle errors in the FFI layer more gracefully and return them to
Swift.

---------

Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Feb 20, 2025
@thomaseizinger
thomaseizinger added this pull request to the merge queue Feb 20, 2025
Merged via the queue into main with commit cad8492 Feb 20, 2025
@thomaseizinger
thomaseizinger deleted the fix/apple-no-panic branch February 20, 2025 01:07
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.

2 participants