getodk / getodk/central

Communicate streamed response errors to clients which don't support HTTP Trailers

Open
#1,253 0 comments 1 reaction 0 assignees View on GitHub
backend
Dominant language
JavaScript
Stars
226
Forks
235
Avg merge
2d 22h
Merged PRs (30d)
19

Description

When there is an error in a streamed response, an HTTP trailer is added at:

https://github.com/getodk/central-backend/blob/7c75951c6b0be9922fe0a8cd853923d81ff94981/lib/http/endpoint.js#L191-L195

From node docs:

> Trailers will **only** be emitted if chunked encoding is used for the response; if it is not (e.g. if the request was HTTP/1.0), they will be silently discarded.
>
> HTTP requires the `Trailer` header to be sent in order to emit trailers,...
> -_https://nodejs.org/docs/latest-v16.x/api/http.html#responseaddtrailersheaders_

There is a unit test covering the error handler: https://github.com/getodk/central-backend/blob/7c75951c6b0be9922fe0a8cd853923d81ff94981/test/unit/http/endpoint.js#L503

It's unclear if the trailers are actually written in practice:

* the test is just checking that `response.addTrailers()` was called
* currently "69%" of browsers support trailers (https://caniuse.com/?search=trailer)
* the `Trailer` header is not being explicitly added to the response
* "chunked encoding" is not being explicitly used for the response (maybe chunked is enabled by default?)

Are the trailers written in normal use? How do they affects e.g. interactive CSV download in a browser?

If the trailers are not written, or a failure is not communicated clearly to users, there might be alternative/additional ways to communicate an error to a user. These might include:

* `.csv`: e.g. write a final line `--- ERROR DOWNLOADING CSV DATA ---`
* `.zip`: e.g. deliberately corrupt the file

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.