Change translation error (500) to not acceptable (406)

[csarven]

The problem is that the server returns a 500 in a situation where it tries to parse a document that it can't parse as RDF, and says "Error translating between RDF formats". This happens when the request (Accept header) includes a media type of a concrete RDF syntax, and the server attempts to parse the document using one of its RDF parsers.

This is not a 500, "unexpected condition" per se. It is an expected condition.

The server should be responding with a 406, not acceptable, because client is asking for a particular representation of the resource (using an RDF media type) and the server is unable to provide one.

I think the flow of code leading up to this can be better / refactored but I didn't want to touch it in this PR.

---

Message to application developers: your application can get around the current issue (500) by including */* (e.g., */*; q=0.1) in Accept, which is a reasonably useful thing to do but YMMV. Your application will then get a 200 representation (whatever the native format of the file/resource is on) instead of a 500. If this PR is merged, your application may get a 406 or 200, which is arguably better to work with than a 500.

[bourgeoa]

What was your use case. Was it text/html to RDFa ?

RDFs are defined here

node-solid-server/lib/ldp.js

Lines 27 to 38 in 83dad06

	constRDF_MIME_TYPES=newSet([
	'text/turtle',// .ttl
	'text/n3',// .n3
	'text/html',// RDFa
	'application/xhtml+xml',// RDFa
	'application/n3',
	'application/nquads',
	'application/n-quads',
	'application/rdf+xml',// .rdf
	'application/ld+json',// .jsonld
	'application/x-turtle'
	])

[csarven]

No, anything, that's not in that list

Try, e.g.:

curl -iH'Accept: text/turtle' https://csarven.solidcommunity.net/public/test.rss

500 response is given with Content-Type: text/plain; charset=utf-8 and "Error translating between RDF formats" in response body.

This is just a case where server can't provide a Turtle representation of that resource. It should be 406.

See also:

curl -iH'Accept: text/turtle, */*; q=0.1' https://csarven.solidcommunity.net/public/test.rss

200 response is given with Content-Type: application/rss+xml

[bourgeoa]

No, anything, that's not in that list

Try, e.g.:

curl -iH'Accept: text/turtle' https://csarven.solidcommunity.net/public/test.rss

Then there is an other issue. 406 should already be used. See

node-solid-server/lib/handlers/get.js

Lines 117 to 121 in 83dad06

	// If it is not in our RDFs we can't even translate,
	// Sorry, we can't help
	if(!possibleRDFType){
	returnnext(error(406,'Cannot serve requested type: '+contentType))
	}

[csarven]

If the selected representation's media type (what's stored on disk) is not in the list of serializable RDF media types, then no need to try to translate, and so should return 406. Only in the case when one of the RDF media types acceptable by the client that's also in the list of serializable RDF media types translation should be attempted.

[bourgeoa]

That's what the actual code tries to do and for to be checked reason is not doing in L117

[csarven]

What do you think of 65b04bb

[bourgeoa]

You can add tests in formats-test.js

 describe('TXT', function (){it('Accept text/plain', function (done){server.get('/put-input.txt') .set('accept', 'text/plain') .expect('Content-type', 'text/plain; charset=utf-8') .expect(200, done) }) it('Accept text/turtle', function (done){server.get('/put-input.txt') .set('accept', 'text/turtle') .expect('Content-type', 'text/plain; charset=utf-8') .expect(406, done) }) }) 

[bourgeoa]

There is the need to change 500 --> 406 in the following test.

node-solid-server/test/integration/acl-oidc-test.js

Lines 745 to 753 in 83dad06

	it('We should have a 500 with invalid group listings',function(done){
	constoptions=createOptions('/group/test-folder/some-other-file.txt','user2')
	request.get(options,function(error,response,body){
	assert.equal(error,null)
	assert.equal(response.statusCode,500)
	done()
	})
	})

[bourgeoa]

@csarven I created a branch translation can you rebase your PR. Thanks

[bourgeoa]

moved to #1755