Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Additional recipes for configuring and deploying the server can be found at [sol
| `--loggingLevel, -l` | `"info"`| |
| `--rootFilePath, -f` | `"./"` | Folder to start the server in when using a file-based config. |
| `--sparqlEndpoint, -s` | | Endpoint to call when using a SPARQL-based config. |
| `--showStackTrace, -t` | false | Whether error stack traces should be shown in responses. |
| `--podConfigJson` | `"./pod-config.json"` | JSON file to store pod configuration when using a dynamic config. |
| `--idpTemplateFolder` | `"templates/idp"` | Folder containing the templates used for IDP interactions. |

Expand Down
1 change: 1 addition & 0 deletions config/example-https-file.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"@id": "urn:solid-server:default:HttpServerFactory",
"@type": "BaseHttpServerFactory",
"handler": { "@id": "urn:solid-server:default:HttpHandler" },
"options_showStackTrace": { "@id": "urn:solid-server:default:variable:showStackTrace" },
"options_https": true,
"options_key": "/path/to/server.key",
"options_cert": "/path/to/server.cert"
Expand Down
1 change: 1 addition & 0 deletions config/http/server-factory/https-example.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"@id": "urn:solid-server:default:HttpServerFactory",
"@type": "BaseHttpServerFactory",
"handler": { "@id": "urn:solid-server:default:HttpHandler" },
"options_showStackTrace": { "@id": "urn:solid-server:default:variable:showStackTrace" },
"options_https": true,
"options_key": "/path/to/server.key",
"options_cert": "/path/to/server.cert"
Expand Down
3 changes: 2 additions & 1 deletion config/http/server-factory/no-websockets.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"comment": "Creates a server that supports HTTP requests.",
"@id": "urn:solid-server:default:ServerFactory",
"@type": "BaseHttpServerFactory",
"handler": { "@id": "urn:solid-server:default:HttpHandler" }
"handler": { "@id": "urn:solid-server:default:HttpHandler" },
"options_showStackTrace": { "@id": "urn:solid-server:default:variable:showStackTrace" }
}
]
}
3 changes: 2 additions & 1 deletion config/http/server-factory/websockets.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"baseServerFactory": {
"@id": "urn:solid-server:default:HttpServerFactory",
"@type": "BaseHttpServerFactory",
"handler": { "@id": "urn:solid-server:default:HttpHandler" }
"handler": { "@id": "urn:solid-server:default:HttpHandler" },
"options_showStackTrace": { "@id": "urn:solid-server:default:variable:showStackTrace" }
},
"webSocketHandler": {
"@type": "UnsecureWebSocketsProtocol",
Expand Down
3 changes: 2 additions & 1 deletion config/identity/handler/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
"providerFactory": { "@id": "urn:solid-server:default:IdentityProviderFactory" },
"interactionPolicy": { "@id": "urn:solid-server:auth:password:AccountInteractionPolicy" },
"interactionHttpHandler": { "@id": "urn:solid-server:auth:password:InteractionHttpHandler" },
"errorResponseWriter": { "@type": "ErrorResponseWriter" }
"errorHandler": { "@id": "urn:solid-server:default:ErrorHandler" },
"responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" }
Copy link
Member

Choose a reason for hiding this comment

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

I hadn't expected this to be at the level of identity, but rather at the level of the server, or some other top handler. So that any error thrown are all handled the same way.

Although I see it's only IDP, so looks like every subsystem (LDP, IDP) just has it own.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree actually, that's what I meant with "extract the preferences at an earlier point" above. It might make sense to extract the preferences in the BaseHttpServerFactory and catch all errors there, instead of doing it in the HttpHandlers. This way we have a single place where error responses are handled. The only reason this was also added now to the IDP is because I previously changed it to use an error responsewriter instead of just returning the error. The StaticAssetHandler doesn't have this yet for example so just throws errors to the server factory.

The one disadvantage would be that then there would be no way for different HttpHandler to have different metadata writers for errors that are thrown. Currently this is not needed yet though.

We could also keep the current solution but then we probably want to, at least for the IDP, find a way to extract preferences there so you don't get a text error while going through the html UI.

Copy link
Member Author

Choose a reason for hiding this comment

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

Additonal confusion: the current BaseHttpServerFactory also has a try/catch for errors which writes out the error stack (which is how the StaticAssetHandler errors get returned). So there is some duplication where errors are handled.

Copy link
Member

@RubenVerborgh RubenVerborgh Jun 4, 2021

Choose a reason for hiding this comment

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

Let's keep it as-is. It keeps the different subsystems independent.

However, let's move (in another PR, or this one)

      const preferences: RepresentationPreferences = { type: { 'text/plain': 1 }};
      const result = await this.errorHandler.handleSafe({ error, preferences });
      await this.responseWriter.handleSafe({ response: input.response, result });

to a single class, which is then the errorWriter that we pass (instead of errorHandler and responseWriter).
That errorWriter can then also have a preferences extractor.


the current BaseHttpServerFactory also has a try/catch for errors which writes out the error stack

That's the (server-specific) emergency solution for when even error handling fails; let's keep that in place.


Just a small note for future reference:

It might make sense to extract the preferences in the BaseHttpServerFactory

This factory should really only make the main server.
Extracting preferences can be done as just an HTTP handler that is reusable with any server.

}
]
}
3 changes: 2 additions & 1 deletion config/identity/handler/provider-factory/identity.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"@type": "IdentityProviderFactory",
"issuer": { "@id": "urn:solid-server:default:variable:baseUrl" },
"configurationFactory": { "@id": "urn:solid-server:default:IdpConfigurationFactory" },
"errorResponseWriter": { "@type": "ErrorResponseWriter" }
"errorHandler": { "@id": "urn:solid-server:default:ErrorHandler" },
"responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" }
},
{
"comment": "Sets up the JWKS and cookie keys.",
Expand Down
21 changes: 21 additions & 0 deletions config/ldp/handler/components/error-handler.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^0.0.0/components/context.jsonld",
"@graph": [
{
"comment": "Changes an error into a valid representation to send as a response.",
"@id": "urn:solid-server:default:ErrorHandler",
"@type": "WaterfallHandler",
"handlers": [
{
"@type": "ConvertingErrorHandler",
"converter": { "@id": "urn:solid-server:default:RepresentationConverter" },
"showStackTrace": { "@id": "urn:solid-server:default:variable:showStackTrace" }
},
{
"@type": "TextErrorHandler",
"showStackTrace": { "@id": "urn:solid-server:default:variable:showStackTrace" }
}
]
}
]
}
10 changes: 2 additions & 8 deletions config/ldp/handler/components/response-writer.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,8 @@
{
"comment": "Writes the result to the response stream.",
"@id": "urn:solid-server:default:ResponseWriter",
"@type": "WaterfallHandler",
"handlers": [
{ "@type": "ErrorResponseWriter" },
{
"@type": "BasicResponseWriter",
"metadataWriter": { "@id": "urn:solid-server:default:MetadataWriter" }
}
]
"@type": "BasicResponseWriter",
"metadataWriter": { "@id": "urn:solid-server:default:MetadataWriter" }
}
]
}
2 changes: 2 additions & 0 deletions config/ldp/handler/default.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^0.0.0/components/context.jsonld",
"import": [
"files-scs:config/ldp/handler/components/error-handler.json",
"files-scs:config/ldp/handler/components/operation-handler.json",
"files-scs:config/ldp/handler/components/request-parser.json",
"files-scs:config/ldp/handler/components/response-writer.json"
Expand All @@ -15,6 +16,7 @@
"args_permissionsExtractor": { "@id": "urn:solid-server:default:PermissionsExtractor" },
"args_authorizer": { "@id": "urn:solid-server:default:Authorizer" },
"args_operationHandler": { "@id": "urn:solid-server:default:OperationHandler" },
"args_errorHandler": { "@id": "urn:solid-server:default:ErrorHandler" },
"args_responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" }
}
]
Expand Down
2 changes: 1 addition & 1 deletion config/ldp/metadata-writer/writers/mapped.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"MappedMetadataWriter:_headerMap_value": "Content-Type"
},
{
"MappedMetadataWriter:_headerMap_key": "urn:solid:http:location",
"MappedMetadataWriter:_headerMap_key": "urn:npm:solid:community-server:http:location",
"MappedMetadataWriter:_headerMap_value": "Location"
}
]
Expand Down
3 changes: 2 additions & 1 deletion config/util/representation-conversion/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"@type": "ChainedConverter",
"converters": [
{ "@id": "urn:solid-server:default:RdfToQuadConverter" },
{ "@id": "urn:solid-server:default:QuadToRdfConverter" }
{ "@id": "urn:solid-server:default:QuadToRdfConverter" },
{ "@type": "ErrorToQuadConverter" }
]
}
]
Expand Down
6 changes: 6 additions & 0 deletions config/util/variables/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,15 @@
"@type": "Variable"
},
{
"comment": "The max level of log messages that should be output.",
"@id": "urn:solid-server:default:variable:loggingLevel",
"@type": "Variable"
},
{
"comment": "Whether error stack traces should be shown in the server responses.",
"@id": "urn:solid-server:default:variable:showStackTrace",
"@type": "Variable"
},
{
"comment": "Path to the JSON file used to store configuration for dynamic pods.",
"@id": "urn:solid-server:default:variable:podConfigJson",
Expand Down
15 changes: 10 additions & 5 deletions src/identity/IdentityProviderFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import type { AnyObject,
Account,
ErrorOut } from 'oidc-provider';
import { Provider } from 'oidc-provider';
import type { ErrorHandler } from '../ldp/http/ErrorHandler';
import type { ResponseWriter } from '../ldp/http/ResponseWriter';

import type { RepresentationPreferences } from '../ldp/representation/RepresentationPreferences';
import type { ConfigurationFactory } from './configuration/ConfigurationFactory';

/**
Expand All @@ -18,13 +19,15 @@ import type { ConfigurationFactory } from './configuration/ConfigurationFactory'
export class IdentityProviderFactory {
private readonly issuer: string;
private readonly configurationFactory: ConfigurationFactory;
private readonly errorResponseWriter: ResponseWriter;
private readonly errorHandler: ErrorHandler;
private readonly responseWriter: ResponseWriter;

public constructor(issuer: string, configurationFactory: ConfigurationFactory,
errorResponseWriter: ResponseWriter) {
errorHandler: ErrorHandler, responseWriter: ResponseWriter) {
this.issuer = issuer;
this.configurationFactory = configurationFactory;
this.errorResponseWriter = errorResponseWriter;
this.errorHandler = errorHandler;
this.responseWriter = responseWriter;
}

public async createProvider(interactionPolicyOptions: {
Expand Down Expand Up @@ -81,7 +84,9 @@ export class IdentityProviderFactory {
},
renderError:
async(ctx: KoaContextWithOIDC, out: ErrorOut, error: Error): Promise<void> => {
await this.errorResponseWriter.handleSafe({ response: ctx.res, result: error });
const preferences: RepresentationPreferences = { type: { 'text/plain': 1 }};
const result = await this.errorHandler.handleSafe({ error, preferences });
await this.responseWriter.handleSafe({ response: ctx.res, result });
},
};
return new Provider(this.issuer, augmentedConfig);
Expand Down
22 changes: 13 additions & 9 deletions src/identity/IdentityProviderHttpHandler.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import type { Provider } from 'oidc-provider';
import type { ErrorHandler } from '../ldp/http/ErrorHandler';
import type { ResponseWriter } from '../ldp/http/ResponseWriter';
import type { RepresentationPreferences } from '../ldp/representation/RepresentationPreferences';
import { getLoggerFor } from '../logging/LogUtil';
import type { HttpHandlerInput } from '../server/HttpHandler';
import { HttpHandler } from '../server/HttpHandler';
import { isNativeError } from '../util/errors/ErrorUtil';
import { assertNativeError, isNativeError } from '../util/errors/ErrorUtil';
import type { IdentityProviderFactory } from './IdentityProviderFactory';
import type { InteractionHttpHandler } from './interaction/InteractionHttpHandler';
import type { InteractionPolicy } from './interaction/InteractionPolicy';
Expand All @@ -26,20 +28,23 @@ export class IdentityProviderHttpHandler extends HttpHandler {
private readonly providerFactory: IdentityProviderFactory;
private readonly interactionPolicy: InteractionPolicy;
private readonly interactionHttpHandler: InteractionHttpHandler;
private readonly errorResponseWriter: ResponseWriter;
private readonly errorHandler: ErrorHandler;
private readonly responseWriter: ResponseWriter;
private provider?: Provider;

public constructor(
providerFactory: IdentityProviderFactory,
interactionPolicy: InteractionPolicy,
interactionHttpHandler: InteractionHttpHandler,
errorResponseWriter: ResponseWriter,
errorHandler: ErrorHandler,
responseWriter: ResponseWriter,
) {
super();
this.providerFactory = providerFactory;
this.interactionPolicy = interactionPolicy;
this.interactionHttpHandler = interactionHttpHandler;
this.errorResponseWriter = errorResponseWriter;
this.errorHandler = errorHandler;
this.responseWriter = responseWriter;
}

/**
Expand Down Expand Up @@ -70,11 +75,10 @@ export class IdentityProviderHttpHandler extends HttpHandler {
try {
await this.interactionHttpHandler.handle({ ...input, provider });
} catch (error: unknown) {
// ResponseWriter can only handle native errors
if (!isNativeError(error)) {
throw error;
}
await this.errorResponseWriter.handleSafe({ response: input.response, result: error });
assertNativeError(error);
const preferences: RepresentationPreferences = { type: { 'text/plain': 1 }};
const result = await this.errorHandler.handleSafe({ error, preferences });
await this.responseWriter.handleSafe({ response: input.response, result });
}
}
}
7 changes: 5 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ export * from './ldp/http/AcceptPreferenceParser';
export * from './ldp/http/BasicRequestParser';
export * from './ldp/http/BasicResponseWriter';
export * from './ldp/http/BodyParser';
export * from './ldp/http/ErrorResponseWriter';
export * from './ldp/http/ConvertingErrorHandler';
export * from './ldp/http/ErrorHandler';
export * from './ldp/http/OriginalUrlExtractor';
export * from './ldp/http/Patch';
export * from './ldp/http/PreferenceParser';
Expand All @@ -119,6 +120,7 @@ export * from './ldp/http/ResponseWriter';
export * from './ldp/http/SparqlUpdateBodyParser';
export * from './ldp/http/SparqlUpdatePatch';
export * from './ldp/http/TargetExtractor';
export * from './ldp/http/TextErrorHandler';

// LDP/Operations
export * from './ldp/operations/DeleteOperationHandler';
Expand Down Expand Up @@ -216,9 +218,10 @@ export * from './storage/accessors/SparqlDataAccessor';
export * from './storage/conversion/ChainedConverter';
export * from './storage/conversion/ConstantConverter';
export * from './storage/conversion/ContentTypeReplacer';
export * from './storage/conversion/ConversionUtil';
export * from './storage/conversion/ErrorToQuadConverter';
export * from './storage/conversion/IfNeededConverter';
export * from './storage/conversion/PassthroughConverter';
export * from './storage/conversion/ConversionUtil';
export * from './storage/conversion/QuadToRdfConverter';
export * from './storage/conversion/RdfToQuadConverter';
export * from './storage/conversion/RepresentationConverter';
Expand Down
49 changes: 21 additions & 28 deletions src/init/AppRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,41 +44,32 @@ export class AppRunner {
} = {}): void {
// Parse the command-line arguments
const { argv: params } = yargs(argv.slice(2))
.strict()
.usage('node ./bin/server.js [args]')
.check((args, options): boolean => {
// Only take flags as arguments, not filenames
if (args._ && args._.length > 0) {
throw new Error(`Unsupported arguments: ${args._.join('", "')}`);
.check((args): boolean => {
if (args._.length > 0) {
throw new Error(`Unsupported positional arguments: "${args._.join('", "')}"`);
}
for (const key in args) {
// Skip filename arguments (_) and the script name ($0)
if (key !== '_' && key !== '$0') {
// Check if the argument occurs in the provided options list
if (!options[key]) {
throw new Error(`Unknown option: "${key}"`);
}
// Check if the argument actually has a value ('> ./bin/server.js -s' is not valid)
if (!args[key]) {
throw new Error(`Missing value for argument "${key}"`);
}
// Check if the argument only has 1 value
if (Array.isArray(args[key])) {
throw new Error(`Multiple values were provided for: "${key}", [${args[key]}]`);
}
for (const key of Object.keys(args)) {
// We have no options that allow for arrays
const val = args[key];
if (key !== '_' && Array.isArray(val)) {
throw new Error(`Multiple values were provided for: "${key}": "${val.join('", "')}"`);
}
}
return true;
})
.options({
baseUrl: { type: 'string', alias: 'b' },
config: { type: 'string', alias: 'c' },
loggingLevel: { type: 'string', alias: 'l', default: 'info' },
mainModulePath: { type: 'string', alias: 'm' },
idpTemplateFolder: { type: 'string' },
port: { type: 'number', alias: 'p', default: 3000 },
rootFilePath: { type: 'string', alias: 'f', default: './' },
sparqlEndpoint: { type: 'string', alias: 's' },
podConfigJson: { type: 'string', default: './pod-config.json' },
baseUrl: { type: 'string', alias: 'b', requiresArg: true },
config: { type: 'string', alias: 'c', requiresArg: true },
loggingLevel: { type: 'string', alias: 'l', default: 'info', requiresArg: true },
mainModulePath: { type: 'string', alias: 'm', requiresArg: true },
idpTemplateFolder: { type: 'string', requiresArg: true },
port: { type: 'number', alias: 'p', default: 3000, requiresArg: true },
rootFilePath: { type: 'string', alias: 'f', default: './', requiresArg: true },
showStackTrace: { type: 'boolean', alias: 't', default: false },
sparqlEndpoint: { type: 'string', alias: 's', requiresArg: true },
podConfigJson: { type: 'string', default: './pod-config.json', requiresArg: true },
})
.help();

Expand Down Expand Up @@ -129,6 +120,7 @@ export class AppRunner {
'urn:solid-server:default:variable:rootFilePath':
this.resolveFilePath(params.rootFilePath),
'urn:solid-server:default:variable:sparqlEndpoint': params.sparqlEndpoint,
'urn:solid-server:default:variable:showStackTrace': params.showStackTrace,
'urn:solid-server:default:variable:podConfigJson':
this.resolveFilePath(params.podConfigJson),
'urn:solid-server:default:variable:idpTemplateFolder':
Expand Down Expand Up @@ -158,6 +150,7 @@ export interface ConfigVariables {
baseUrl?: string;
rootFilePath?: string;
sparqlEndpoint?: string;
showStackTrace?: boolean;
podConfigJson?: string;
idpTemplateFolder?: string;
}
Loading