Repository: solid/community-server (ts)

Issue:
Create nice error writers
We should have content-negotiable error writers, (partially) reusing the infrastructure we currently have for content-negotiated responses.

This will enable nicer error messages.

A particular case is just serving Databrowser in case of 403.

Pull request:
feat: Add ErrorToHtmlConverter using templates
Closes #654.

Will also have to change after #721 is resolved.

Patch:
diff --git a/config/util/representation-conversion/default.json b/config/util/representation-conversion/default.json
index 8c7b2835c..3c23ae87b 100644
--- a/config/util/representation-conversion/default.json
+++ b/config/util/representation-conversion/default.json
@@ -24,7 +24,13 @@
           "converters": [
             { "@id": "urn:solid-server:default:RdfToQuadConverter" },
             { "@id": "urn:solid-server:default:QuadToRdfConverter" },
-            { "@type": "ErrorToQuadConverter" }
+            { "@type": "ErrorToQuadConverter" },
+            {
+              "@type": "ErrorToTemplateConverter",
+              "engine": { "@type": "HandlebarsTemplateEngine" },
+              "templatePath": "$PACKAGE_ROOT/templates/error/error.hbs",
+              "contentType": "text/html"
+            }
           ]
         }
       ]
diff --git a/src/index.ts b/src/index.ts
index 13408156f..a748f87f6 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -220,6 +220,7 @@ export * from './storage/conversion/ChainedConverter';
 export * from './storage/conversion/ConstantConverter';
 export * from './storage/conversion/ContentTypeReplacer';
 export * from './storage/conversion/ConversionUtil';
+export * from './storage/conversion/ErrorToTemplateConverter';
 export * from './storage/conversion/ErrorToQuadConverter';
 export * from './storage/conversion/IfNeededConverter';
 export * from './storage/conversion/PassthroughConverter';
diff --git a/src/storage/conversion/ErrorToTemplateConverter.ts b/src/storage/conversion/ErrorToTemplateConverter.ts
new file mode 100644
index 000000000..d36e9649d
--- /dev/null
+++ b/src/storage/conversion/ErrorToTemplateConverter.ts
@@ -0,0 +1,43 @@
+import { promises as fsPromises } from 'fs';
+import arrayifyStream from 'arrayify-stream';
+import { BasicRepresentation } from '../../ldp/representation/BasicRepresentation';
+import type { Representation } from '../../ldp/representation/Representation';
+import type { TemplateEngine } from '../../pods/generate/TemplateEngine';
+import { INTERNAL_ERROR } from '../../util/ContentTypes';
+import { InternalServerError } from '../../util/errors/InternalServerError';
+import { resolveAssetPath } from '../../util/PathUtil';
+import type { RepresentationConverterArgs } from './RepresentationConverter';
+import { TypedRepresentationConverter } from './TypedRepresentationConverter';
+
+/**
+ * Serializes an Error by filling in the provided template.
+ * Content-type is based on the constructor parameter.
+ */
+export class ErrorToTemplateConverter extends TypedRepresentationConverter {
+  private readonly engine: TemplateEngine;
+  private readonly templatePath: string;
+  private readonly contentType: string;
+
+  public constructor(engine: TemplateEngine, templatePath: string, contentType: string) {
+    super(INTERNAL_ERROR, contentType);
+    this.engine = engine;
+    this.templatePath = resolveAssetPath(templatePath);
+    this.contentType = contentType;
+  }
+
+  public async handle({ representation }: RepresentationConverterArgs): Promise<Representation> {
+    const errors = await arrayifyStream(representation.data);
+    if (errors.length !== 1) {
+      throw new InternalServerError('Only single errors are supported.');
+    }
+    const error = errors[0] as Error;
+
+    // Render the template
+    const { name, message, stack } = error;
+    const variables = { name, message, stack };
+    const template = await fsPromises.readFile(this.templatePath, 'utf8');
+    const html = this.engine.apply(template, variables);
+
+    return new BasicRepresentation(html, representation.metadata, this.contentType);
+  }
+}
diff --git a/src/util/ContentTypes.ts b/src/util/ContentTypes.ts
index 40cd83776..9163ab17d 100644
--- a/src/util/ContentTypes.ts
+++ b/src/util/ContentTypes.ts
@@ -1,9 +1,10 @@
 // Well-known content types
-export const TEXT_TURTLE = 'text/turtle';
 export const APPLICATION_JSON = 'application/json';
 export const APPLICATION_OCTET_STREAM = 'application/octet-stream';
 export const APPLICATION_SPARQL_UPDATE = 'application/sparql-update';
 export const APPLICATION_X_WWW_FORM_URLENCODED = 'application/x-www-form-urlencoded';
+export const TEXT_HTML = 'text/html';
+export const TEXT_TURTLE = 'text/turtle';
 
 // Internal content types (not exposed over HTTP)
 export const INTERNAL_ALL = 'internal/*';
diff --git a/templates/error/error.hbs b/templates/error/error.hbs
new file mode 100644
index 000000000..26dcb9f23
--- /dev/null
+++ b/templates/error/error.hbs
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta charset="utf-8"/>
+    <meta name="viewport" content="width=device-width, initial-scale=1"/>
+    <title>{{ name }}</title>
+  </head>
+  <body>
+    <h1>{{ name }}</h1>
+    <p>{{ message }}</p>
+    {{#if stack}}
+      <pre><code>{{ stack }}</code></pre>
+    {{/if}}
+  </body>
+</html>


Test patch:
diff --git a/test/unit/storage/conversion/ErrorToTemplateConverter.test.ts b/test/unit/storage/conversion/ErrorToTemplateConverter.test.ts
new file mode 100644
index 000000000..955425493
--- /dev/null
+++ b/test/unit/storage/conversion/ErrorToTemplateConverter.test.ts
@@ -0,0 +1,75 @@
+import { BasicRepresentation } from '../../../../src/ldp/representation/BasicRepresentation';
+import type { TemplateEngine } from '../../../../src/pods/generate/TemplateEngine';
+import { ErrorToTemplateConverter } from '../../../../src/storage/conversion/ErrorToTemplateConverter';
+import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError';
+import { InternalServerError } from '../../../../src/util/errors/InternalServerError';
+import { readableToString } from '../../../../src/util/StreamUtil';
+
+const mockRead = jest.fn().mockResolvedValue('{{ template }}');
+jest.mock('fs', (): any => ({
+  promises: { readFile: (...args: any[]): any => mockRead(...args) },
+}));
+
+describe('An ErrorToTemplateConverter', (): void => {
+  const identifier = { path: 'http://test.com/error' };
+  let engine: TemplateEngine;
+  const path = '/template/error.template';
+  let converter: ErrorToTemplateConverter;
+  const preferences = {};
+
+  beforeEach(async(): Promise<void> => {
+    mockRead.mockClear();
+    engine = {
+      apply: jest.fn().mockReturnValue('<html>'),
+    };
+
+    converter = new ErrorToTemplateConverter(engine, path, 'text/html');
+  });
+
+  it('supports going from errors to quads.', async(): Promise<void> => {
+    await expect(converter.getInputTypes()).resolves.toEqual({ 'internal/error': 1 });
+    await expect(converter.getOutputTypes()).resolves.toEqual({ 'text/html': 1 });
+  });
+
+  it('does not support multiple errors.', async(): Promise<void> => {
+    const representation = new BasicRepresentation([ new Error(), new Error() ], 'internal/error', false);
+    const prom = converter.handle({ identifier, representation, preferences });
+    await expect(prom).rejects.toThrow('Only single errors are supported.');
+    await expect(prom).rejects.toThrow(InternalServerError);
+  });
+
+  it('calls the template engine with all error fields.', async(): Promise<void> => {
+    const error = new BadRequestHttpError('error text');
+    const representation = new BasicRepresentation([ error ], 'internal/error', false);
+    const prom = converter.handle({ identifier, representation, preferences });
+    await expect(prom).resolves.toBeDefined();
+    const result = await prom;
+    expect(result.binary).toBe(true);
+    expect(result.metadata.contentType).toBe('text/html');
+    await expect(readableToString(result.data)).resolves.toBe('<html>');
+    expect(mockRead).toHaveBeenCalledTimes(1);
+    expect(mockRead).toHaveBeenLastCalledWith(path, 'utf8');
+    expect(engine.apply).toHaveBeenCalledTimes(1);
+    expect(engine.apply).toHaveBeenLastCalledWith(
+      '{{ template }}', { name: 'BadRequestHttpError', message: 'error text', stack: error.stack },
+    );
+  });
+
+  it('only adds stack if it is defined.', async(): Promise<void> => {
+    const error = new BadRequestHttpError('error text');
+    delete error.stack;
+    const representation = new BasicRepresentation([ error ], 'internal/error', false);
+    const prom = converter.handle({ identifier, representation, preferences });
+    await expect(prom).resolves.toBeDefined();
+    const result = await prom;
+    expect(result.binary).toBe(true);
+    expect(result.metadata.contentType).toBe('text/html');
+    await expect(readableToString(result.data)).resolves.toBe('<html>');
+    expect(mockRead).toHaveBeenCalledTimes(1);
+    expect(mockRead).toHaveBeenLastCalledWith(path, 'utf8');
+    expect(engine.apply).toHaveBeenCalledTimes(1);
+    expect(engine.apply).toHaveBeenLastCalledWith(
+      '{{ template }}', { name: 'BadRequestHttpError', message: 'error text' },
+    );
+  });
+});