-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Streamablefile throws error for range requests on iOS when playing audio file #14873
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
Comments
I didn't manage to reproduce the error in iOS 16 (safari). |
I believe that this is just a limitation of |
I tried similar code on iOS 15.8 on real device and faced the issue. |
@jmcdo29 is there any scenario when |
Let's track this here #15010 |
@sudarsangp can you please test in your side if this bug is addressed by this minimal change: https://github.com/nestjs/nest/pull/15010/files |
@micalevisk sure. Can you tell me how to test this? I tried replacing |
@sudarsangp you can just navigate to your local node_modules and apply those changes manually (should be very simple given the size of this PR) |
@kamilmysliwiec sure. Existing code in return (0, stream_1.pipeline)(body.getStream().once('error', (err) => {
body.errorHandler(err, response);
}), response, (err) => {
if (err) {
body.errorLogger(err);
}
}); Updated code (based on PR) return body.getStream().once('error', (err) => {
body.errorHandler(err, response);
}), response, (err) => {
if (err) {
body.errorLogger(err);
}
}; Can you help confirm if this is correct? If yes, this results in timeout being exceeded for the tests. |
Thanks for the quick turnaround @kamilmysliwiec With the below code the tests do pass. stream_1.pipeline(
body.getStream().once('error', (err) => {
body.errorHandler(err, response);
}), response, (err) => {
if (err) {
body.errorLogger(err);
}
});
const stream = body.getStream();
stream.once('error', err => {
body.errorHandler(err, response);
});
return stream.pipe(response)
.on('error', (err) => body.errorLogger(err)); |
Is there an existing issue for this?
Current behavior
When a range request is made from iOS device I get this error:
When trying to debug the code using debugger, found this thrown from
express-adapter.js
file, theerr
object is shown here:There were cases when the err object was empty and it didn't thrown an error due to this code:
I am not able to catch this error using catch everything exception filter
The code which throws the error is here:
Any suggestions on how to resolve this error?
Is there any workaround at-least to catch the error?
Minimum reproduction code
https://stackblitz.com/edit/nestjs-typescript-starter-adjltria?file=test%2Fapp.e2e-spec.ts
Steps to reproduce
I am not sure if this is the correct way to reproduce though:
npm run test:e2e
Expected behavior
A way for error to be caught and not kill the entire backend server.
The reason being we cannot control how client makes a request.
Package
@nestjs/common
@nestjs/core
@nestjs/microservices
@nestjs/platform-express
@nestjs/platform-fastify
@nestjs/platform-socket.io
@nestjs/platform-ws
@nestjs/testing
@nestjs/websockets
Other package
No response
NestJS version
^10.0.0
Packages versions
Node.js version
v18.17.0
In which operating systems have you tested?
Other
The issue happens when trying to play the audio on iOS.
Additional context here - https://discord.com/channels/695411232856997968/1354845020548763819/1354845020548763819
The text was updated successfully, but these errors were encountered: