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

Document custom request size limits with Multipart #3494

Merged
merged 5 commits into from
Apr 14, 2021
Merged

Document custom request size limits with Multipart #3494

merged 5 commits into from
Apr 14, 2021

Conversation

tobias-tengler
Copy link
Collaborator

@tobias-tengler tobias-tengler commented Apr 13, 2021

I added a note about request size limits to the documentation.

I also changed one of the existing multipart error messages to include the exception detailing why the parsing of the multipart form failed. This should help debugging issues related to request sizes and badly formed multipart requests.

Relates to #3473

@tobias-tengler tobias-tengler marked this pull request as ready for review April 13, 2021 16:31
michaelstaib
michaelstaib previously approved these changes Apr 14, 2021
@michaelstaib michaelstaib added 🌶️ hot chocolate 📚 documentation This issue is about working on our documentation. labels Apr 14, 2021
@michaelstaib michaelstaib added this to the HC-11.2.0 milestone Apr 14, 2021
@michaelstaib
Copy link
Member

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@tobias-tengler
Copy link
Collaborator Author

Oh, I think I forgot to run tests again, after including the Exception. I will fix it!

@tobias-tengler
Copy link
Collaborator Author

@michaelstaib I thought adding the exception to the ErrorBuilder would include it in the output, but it isn't inlcuded:
image
How would I include the original exception? I think this would give meaningful insights into how to fix the issue (at least in development).

@michaelstaib
Copy link
Member

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@michaelstaib
Copy link
Member

Adding the exception will make the exception available to internal diagnostics. This means that if you attach a logger or intercept the error you will be able to catch it. If you want to expose the internal error message you should also add an extension field to the error:

ErrorBuilder.New()
   ....
   .SetExtension("underlyingError", ex.Message)

Just add this additionally.

@tobias-tengler
Copy link
Collaborator Author

Added the extension field and also added a test that showcases what happens if a file is uploaded that exceeds limits.

@michaelstaib
Copy link
Member

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@michaelstaib
Copy link
Member

@tobias-tengler I have given you write access to the repo, you can now add your own branches to the main repo which makes contributing easier.

The branch names are structured like follows: {3 letters representing your name}/{branch name}.

example mst/some-feature.

@sonarcloud
Copy link

sonarcloud bot commented Apr 14, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@michaelstaib michaelstaib merged commit 33ecb51 into ChilliCream:main Apr 14, 2021
@tobias-tengler tobias-tengler deleted the patch-1 branch April 14, 2021 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation This issue is about working on our documentation. 🌶️ hot chocolate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants