Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Allow error reason to get to the server #3150

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Gigitsu
Copy link

@Gigitsu Gigitsu commented Mar 4, 2024

This PR aims to allow external upload error reason to get to the server. This will enable the server to render error messages in a live view, providing a more seamless and informative user experience.

See this post for more details.

Please be aware that this PR introduces a breaking change. However, I think it is necessary to ensure that the argument passed to the UploadEntity error function remains useful in the context of external uploads.

@SteffenDE
Copy link
Collaborator

Related: #2502

#1478 (comment)

@chrismccord
Copy link
Member

Note my hesitancy here is a few things - the value cannot be trusted from the client, the value is held in memory, and there is no way to "reactively" handle the error, ie it is only really useful in the template. To really handle it you need to go through one of the LiveView callbacks, in which case you need to message the LV out of band. I do think passing as a tuple is reasonable, as it indicates it comes from the client. We could also guard on bytesize, but we'd mixing trusted values vs untrsusted values, so I am not sure. In practice you'll also need to handle this yourself by pushing a message anyway. In your usecase, are you simply wanting to show the error message you got from S3 on the UI somewhere? Or are you doing more with the error value?

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.

None yet

3 participants