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

fix: AssetInterceptor "can't set headers after they are sent" #9987

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

midzelis
Copy link
Contributor

@midzelis midzelis commented Jun 4, 2024

This fixes a problem in the interceptor. It was calling send() on the underlying express response, which caused it to send the body. However, it did not prevent the downstream interceptor/controller from running, as was intended. And then later, when the downstream response was trying to be sent to client, protocol errors occurred.

The symptoms are like this:

immich_web               | Error: write EPIPE
immich_web               |     at afterWriteDispatched (node:internal/stream_base_commons:161:15)
immich_web               |     at writeGeneric (node:internal/stream_base_commons:152:3)
immich_web               |     at Socket._writeGeneric (node:net:953:11)
immich_web               |     at Socket._write (node:net:965:8)
immich_web               |     at doWrite (node:internal/streams/writable:590:12)
immich_web               |     at clearBuffer (node:internal/streams/writable:774:7)
immich_web               |     at Writable.uncork (node:internal/streams/writable:523:7)
immich_web               |     at connectionCorkNT (node:_http_outgoing:981:8)
immich_web               |     at process.processTicksAndRejections (node:internal/process/task_queues:81:21)
immich_web               | 11:00:20 AM [vite] http proxy error: /api/assets/
immich_web               | Error: write EPIPE
immich_web               |     at WriteWrap.onWriteComplete [as oncomplete] (node:internal/stream_base_commons:95:16)
immich_web               |     at WriteWrap.callbackTrampoline (node:internal/async_hooks:130:17)

in production builds manifest a bit different

immich-e2e-server  | [Nest] 7  - 06/04/2024, 10:29:03 PM   ERROR [ImmichServer] [ExceptionsHandler] Cannot set headers after they are sent to the client
immich-e2e-server  | Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
immich-e2e-server  |     at ServerResponse.setHeader (node:_http_outgoing:659:11)
immich-e2e-server  |     at ServerResponse.header (/usr/src/app/node_modules/express/lib/response.js:795:10)
immich-e2e-server  |     at ServerResponse.send (/usr/src/app/node_modules/express/lib/response.js:175:12)
immich-e2e-server  |     at ServerResponse.json (/usr/src/app/node_modules/express/lib/response.js:279:15)
immich-e2e-server  |     at ExpressAdapter.reply (/usr/src/app/node_modules/@nestjs/platform-express/adapters/express-adapter.js:62:62)
immich-e2e-server  |     at RouterResponseController.apply (/usr/src/app/node_modules/@nestjs/core/router/router-response-controller.js:15:36)
immich-e2e-server  |     at /usr/src/app/node_modules/@nestjs/core/router/router-execution-context.js:176:48
immich-e2e-server  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
immich-e2e-server  |     at async /usr/src/app/node_modules/@nestjs/core/router/router-execution-context.js:47:13
immich-e2e-server  |     at async /usr/src/app/node_modules/@nestjs/core/router/router-proxy.js:9:17

@midzelis
Copy link
Contributor Author

midzelis commented Jun 4, 2024

Linkify to #9229

res.status(200);
// Returning the response body wrapped in an Observable will cause nestjs
// to use it, and skip all downstream request processing.
return of({ status: AssetMediaStatus.DUPLICATE, id: response.id });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the of() method do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes an observable out of the object.

@alextran1502
Copy link
Contributor

alextran1502 commented Jun 5, 2024

With this change, you don't get the duplicated status when uploading a duplicated asset. You can reproduce by dragging and dropping the same file on the web to upload it

Hold on, something is wrong on main

@jrasm91 jrasm91 merged commit ce985ef into immich-app:main Jun 5, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants