Skip to content

fix: crash when opening null url#2134

Merged
khushal87 merged 2 commits intoGetStream:developfrom
arthurgeron:fix/video-attachment-crash
Oct 31, 2023
Merged

fix: crash when opening null url#2134
khushal87 merged 2 commits intoGetStream:developfrom
arthurgeron:fix/video-attachment-crash

Conversation

@arthurgeron-work
Copy link
Copy Markdown
Contributor

🎯 Goal

Fix the application crash that happens when opening a video attachment before the upload is finished.

πŸ›  Implementation details

Fixes #2133
Avoids sending a null/undefined URL to the native video loader, which would cause a crash.

🎨 UI Changes

iOS
Before After
N/A N/A
Android
Before After
N/A N/A

πŸ§ͺ Testing

1- Open a chat
2- Open the attachment picker and select a video
3- Send the video
4- Immediately click on the new video thumbnail in the chat before the upload finishes (it'll be black)
Should not crash with this fix.

β˜‘οΈ Checklist

  • I have signed the Stream CLA (required)
  • PR targets the develop branch
  • Documentation is updated
  • New code is tested in main example apps, including all possible scenarios
    • SampleApp iOS and Android
    • Expo iOS and Android

Arthur Geron added 2 commits June 2, 2023 13:02
previous if would fail the validation for non-video attachments
Copy link
Copy Markdown
Member

@santhoshvai santhoshvai left a comment

Choose a reason for hiding this comment

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

@khushal87 This looks good to me.. but can you check this as well please, being the video owner

@khushal87
Copy link
Copy Markdown
Contributor

The change is fine, but honestly, I have not been able to reproduce this issue at all on my end even after multiple tries of uploading video and opening them, that is why I am not sure if it is really needed.

@arthurgeron-work
Copy link
Copy Markdown
Contributor Author

arthurgeron-work commented Oct 25, 2023

The change is fine, but honestly, I have not been able to reproduce this issue at all on my end even after multiple tries of uploading video and opening them, that is why I am not sure if it is really needed.

@khushal87 You'll find a recording which shows the reproduction of this exception at the bottom. I was able to reproduce it as soon as I reverted this fix locally.

Despite not being able to easily replicate the issue on your side, I contend that this pull request (PR) holds significant value in ensuring optimal behavior. In particular, if we are dealing with a video and there is no thumbnail URL present, it is logically sound not to attempt opening the file since its non-availability implies it is premature to do so.

Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2023-10-25.at.12.34.11.mp4

@khushal87 khushal87 merged commit 660c19d into GetStream:develop Oct 31, 2023
@github-actions github-actions Bot mentioned this pull request Nov 6, 2023
khushal87 added a commit that referenced this pull request Nov 6, 2023
* feat: apply theme to SendButton internal icons (#2280)

* fix: crash when opening null url (#2134)

* fix: crash when opening null url

* fix: only check url if type is video
previous if would fail the validation for non-video attachments

---------

Co-authored-by: Arthur Geron <arthurgeron@hotmail.com>

* feat: upgrade axios to v1 (#2281)

* fix: multiple mentions render in message text (#2286)

* chore: replace last_message_at with last_updated (#2282)

* chore: replace last_message_at with last_updated

* fix: keep the same order as before

* fix(expo): do not show reconnecting status while showing gallery

---------

Co-authored-by: Khushal Agarwal <khushal.agarwal987@gmail.com>
Co-authored-by: Arthur Geron <71399856+arthurgeron-work@users.noreply.github.com>
Co-authored-by: Arthur Geron <arthurgeron@hotmail.com>
Co-authored-by: Vishal Narkhede <vishalnarkhede.iitd@gmail.com>
@github-actions github-actions Bot mentioned this pull request Nov 6, 2023
@stream-ci-bot
Copy link
Copy Markdown
Contributor

πŸŽ‰ This PR is included in version 5.20.0 πŸŽ‰

The release is available on:

Your semantic-release bot πŸ“¦πŸš€

@arthurgeron arthurgeron deleted the fix/video-attachment-crash branch June 18, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

πŸ”₯[πŸ›] React Native - Crash on opening video before upload

4 participants