zotero / translation-server

A Node.js-based server to run Zotero translators
Other
122 stars 51 forks source link

Handles remote 4xx and 5xx responses and logs them #171

Open foldleft opened 1 month ago

foldleft commented 1 month ago

At present if the remote service returns an error we get an undifferentiated 500 back from translation-server. This means we cannot distinguish between a 403, a 404 or a 500. This change allows us to log what happened.

Testing

I created a directory called nginx-force-response and populated conf.d/default.conf with a file like this (but with more entries):

server {
  listen 80;
  server_name localhost;
  location /401 {
    return 401;
  }
  location /501 {
    return 501;
  }
}

And then used it like so: docker run --rm --name nginx -v $PWD/conf.d:/etc/nginx/conf.d -p 8080:80 nginx

Finally, I was able to test like this:

$ curl -d 'http://127.0.0.1:8080/501' -H "content-type: text/plain" http://localhost:1969/web && echo
Remote server encountered an error handling our request (501)
$ curl -d 'http://127.0.0.1:8080/401' -H "content-type: text/plain" http://localhost:1969/web && echo
Remote server could not provide document (401)
mvolz commented 1 month ago

I definitely agree a 500 doesn't make sense here for these kinds of errors.

I'm not sure a 501 makes sense because I think of that as being a server to handle the request in the service, not one upstream... would something in the 4xx make more sense/ be something Zotero would be willing to merge?

foldleft commented 2 weeks ago

I think 500 or 503 are appropriate – it's not quite acting as a gateway, but 503 does get the sense of the error being because of a downstream service.

The 501 there is just a temporary nginx configuration I set up for testing.