refactor(server): file streaming logic

[mertalev]

Description

Switching to Express's sendFile results in much cleaner code that sets appropriate headers automatically. The current approach is too manual and error-prone in the absence of a good reason for it.

Fixes #3097

How Has This Been Tested?

Tested on web by scrolling, hovering on videos and live photos and switching between content in the expanded view. All content is shown as expected and no errors are reported in the server.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
immich	⬜️ Ignored (Inspect)			Jul 13, 2023 4:15pm

[alextran1502]

The issue I encountered was on Android. The media player required those range properties to be sent. Can you please test on Android and make sure that you can stream the video?

[mertalev]

I don't have an Android phone, but sendFile should handle ranges if they're sent in the request header.

[alextran1502]

I can help testing later

[uhthomas]

This is great, thank you for doing this.

[mertalev]

Alright, so Nest-style exceptions don't work at all when passthrough is disabled. They leave everything up to you, so throwing an exception actually crashes the server. On the other hand, enabling it also doesn't work because the response is already sent and there's nothing to passthrough. I'm not sure if there's a good way to fix this. I'm all ears if there is, but otherwise I'll have to shelve this.

[jrasm91]

Maybe we can use a custom interceptor for these types of requests. Basically automatically catch the exception and map it to a valid response.

I think you can get the http adapter and call reply potentially..

In the controller we probably want to add a .catch(...) and handle the error either via @Next() or httpAdapter.reply()

[mertalev]

Adding an interceptor or filter seems to just get ignored. But adding .catch to the call gets the job done:

this.assetService.serveFile(authUser, id, query, res).catch((err: Error) => {
  let status = 500;
  let body = err.message;
  if (err instanceof HttpException) {
    status = err.getStatus();
    body = JSON.stringify(err.getResponse());
  }
  return res.status(status).end(body);
});

[jrasm91]

This is probably good enough for now. We're basically copying the implementation of the default/base nestjs exception filter here though. Might be worth seeing if it is possible to call the filter.catch method manually though, instead of duplicating the logic.

[mertalev]

Exception handling works now without hacks. I.. I just needed to return...

[alextran1502]

Hello, can you help me resolve the merge conflict?

[mertalev]

Rebased

[alextran1502]

I tested on Android device Samsung S9 and the video file cannot be played. The same issue I encountered originally thus the implementation of setResRange

Edit: Actually S8 is ancient and something cause the video cannot be played on main as well. @jrasm91 Do you mind help me test this on your Android phone?

[jrasm91]

I can test it tomorrow.

cwd is always /usr/src/app

[jrasm91]

Tested on the web and mobile. Everything looks good.

• Videos return 206 with byte ranges
• Seeking/buffer works
• Images and thumbnails are served
• premature close warning is gone from the logs