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[bot]]

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

[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

[jrasm91]

I can test it tomorrow.