Skip to content

Conversation

@denrase
Copy link
Collaborator

@denrase denrase commented Feb 19, 2025

📜 Description

The source maps reference sources from the root folder. So when we added lib as a folder for sources to upload, we also need to add lib as a prefix so the reference is correct.

💡 Motivation and Context

Closes #299
Closes #304

💚 How did you test it?

Updated unit test.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

Comment on lines +236 to +240
['dart'],
releaseDartFilesParams,
release,
'lib',
'~/lib/',
Copy link
Contributor

Choose a reason for hiding this comment

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

do you know what happens for packages relying on other packages?

e.g I have a main app that consumes internal modules A and B

I guess this would only cover uploading and prefixing files correctly for main app so frames coming from module A and B would not have source context?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As long as the modules are within the lib folder, it "should" be fine. If they are outside, we are not supporting this currently, as we intentionally moved away from scanning the whole folder.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that can be a usecase though right? when they have internal libraries that they consume for example. we don't have to support it now but I guess we can create an issue and see if people are interested

Copy link
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

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

looks good 👍 we should also think about writing proper end-to-end tests for this in the future

@denrase denrase marked this pull request as draft February 19, 2025 14:44
@denrase
Copy link
Collaborator Author

denrase commented Feb 19, 2025

@buenaflor Put this in draft as i want to test more scenarios when we get feedback.

@buenaflor
Copy link
Contributor

change looks good to me

@denrase denrase marked this pull request as ready for review February 21, 2025 13:34
@denrase denrase merged commit e0e67a4 into main Feb 21, 2025
18 of 19 checks passed
@denrase denrase deleted the fix-missing-source-context branch February 21, 2025 13:34
@github-actions
Copy link
Contributor

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

- Add missing prefix to source file upload ([#306](https://github.com/getsentry/sentry-dart-plugin/pull/306))

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 06f1deb

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.

Flutter Web: Missing Sources Context on sentry platform Source context not applied to .dart stack traces

3 participants