这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@philipphofmann
Copy link
Member

📜 Description

Logging an error when getting all files for a folder that doesn't exist confuses users; see GH-3577. Instead, log an info message that the SentryFileManager tried getting a list of files for a folder that doesn't exist. We chose info because it's still an edge case that shouldn't occur often, but it doesn't break any functionality. If a path was deleted manually by the user that the SDK requires, the SDK will create the path on next launch. We don't want to add checks if all the necessary paths exist every time the SDK stores an envelope cause this adds some overhead. If users manually remove the required paths, we accept that they have to wait until the next time the SDK launches.

💡 Motivation and Context

Fixes GH-3577

💚 How did you test it?

Unit tests.

📝 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

Logging an error when getting all files for a folder that doesn't exist
confuses users; see GH-3577. Instead, log an info message that the
SentryFileManager tried getting a list of files for a folder that
doesn't exist. We chose info because it's still an edge case that
shouldn't occur often, but it doesn't break any functionality. If a path
was deleted manually by the user that the SDK requires, the SDK will
create the path on next launch. We don't want to add checks if all the
necessary paths exist every time the SDK stores an envelope cause this
adds some overhead. If users manually remove the required paths, we
accept that they have to wait until the next time the SDK launches.
@codecov
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (52e4912) 89.275% compared to head (9e1c19b) 89.275%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3594       +/-   ##
=============================================
- Coverage   89.275%   89.275%   -0.001%     
=============================================
  Files          529       529               
  Lines        57793     57765       -28     
  Branches     20682     20664       -18     
=============================================
- Hits         51595     51570       -25     
- Misses        5174      5283      +109     
+ Partials      1024       912      -112     
Files Coverage Δ
...ts/SentryTests/Helper/SentryFileManagerTests.swift 97.575% <100.000%> (+0.056%) ⬆️
Sources/Sentry/SentryFileManager.m 93.167% <66.666%> (-0.570%) ⬇️

... and 42 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 52e4912...9e1c19b. Read the comment docs.

@github-actions
Copy link
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1194.53 ms 1210.67 ms 16.14 ms
Size 21.58 KiB 418.29 KiB 396.71 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
a2af9fa 1236.62 ms 1253.12 ms 16.50 ms
f25febb 1209.08 ms 1225.90 ms 16.81 ms
7cd187e 1239.02 ms 1261.42 ms 22.40 ms
9cc7e7c 1228.90 ms 1237.96 ms 9.06 ms
3cb68af 1221.15 ms 1238.40 ms 17.25 ms
881a955 1222.16 ms 1237.22 ms 15.06 ms
5b6694b 1221.71 ms 1259.06 ms 37.35 ms
66922ca 1221.68 ms 1235.98 ms 14.30 ms
42ef6ba 1195.04 ms 1214.35 ms 19.31 ms
279841c 1231.41 ms 1245.65 ms 14.24 ms

App size

Revision Plain With Sentry Diff
a2af9fa 20.76 KiB 432.87 KiB 412.11 KiB
f25febb 21.58 KiB 414.92 KiB 393.34 KiB
7cd187e 20.76 KiB 401.65 KiB 380.89 KiB
9cc7e7c 22.84 KiB 403.13 KiB 380.29 KiB
3cb68af 20.76 KiB 401.60 KiB 380.84 KiB
881a955 22.85 KiB 407.63 KiB 384.78 KiB
5b6694b 20.76 KiB 426.11 KiB 405.34 KiB
66922ca 20.76 KiB 425.80 KiB 405.04 KiB
42ef6ba 21.58 KiB 417.87 KiB 396.28 KiB
279841c 22.84 KiB 403.19 KiB 380.35 KiB

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.

LGTM

@philipphofmann philipphofmann merged commit 86f7249 into main Jan 31, 2024
@philipphofmann philipphofmann deleted the fix/dont-log-error-when-no-envelopes-path branch January 31, 2024 07:57
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.

Could not get NSFileTypeDirectory, Couldn't load files in folder [envelopes]

3 participants