-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: buffer as response from API #36009
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
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: ee0d480 The changes in this PR will be included in the next version bump. This PR includes changesets to release 36 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #36009 +/- ##
===========================================
+ Coverage 64.69% 64.71% +0.02%
===========================================
Files 3248 3248
Lines 95318 95329 +11
Branches 17894 17905 +11
===========================================
+ Hits 61665 61694 +29
+ Misses 30747 30734 -13
+ Partials 2906 2901 -5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request fixes an issue where the API response for app icons was not returning a proper buffer, causing runtime errors. Key changes include:
- Converting the Content-Length header to a string in the API response.
- Updating test cases in router.spec.ts to validate expected response schemas.
- Enhancing the honoAdapter middleware to correctly handle responses with different body types.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/meteor/ee/server/apps/communication/rest.ts | Converts the Content-Length to a string to avoid type errors. |
| apps/meteor/app/api/server/router.spec.ts | Updates tests to ensure valid response bodies and schema validation. |
| apps/meteor/app/api/server/middlewares/honoAdapter.ts | Adds early return for missing body and handles ReadableStream responses. |
| apps/meteor/app/api/server/definition.ts | Extends the ResultFor type to include an optional headers property. |
| .changeset/polite-islands-try.md | Documents the fix regarding buffer responses for app icons. |
|
/backport 7.6.1 |
|
Sorry, I couldn't do that backport because of conflicts. Could you please solve them? you can do so by running the following commands: after that just run |
Co-authored-by: Guilherme Gazzo <[email protected]> Co-authored-by: Guilherme Gazzo <[email protected]>
|
/backport 7.6.1 |
|
Pull request #36060 added to Project: "Patch 7.6.1" |
https://rocketchat.atlassian.net/browse/ARCH-1607
Proposed changes (including videos or screenshots)
The following error was being thrown when showing an app's icon like
http://localhost:3000/api/apps/c33fa1a6-68a7-491e-bf49-9d7b99671c48/iconIssue(s)
Steps to test or reproduce
Further comments