Customizable Error Filtering for js-rpc through User-Defined toError/fromError Functions

[addievo]

Description

Enable user-defined error serialization and deserialization through toError and fromError functions in js-rpc. Add optional replacer and reviver parameters to further customize the (de)serialization process, thereby enhancing security by allowing application-specific rules for sensitive data filtering.

Requirements

1. User-Driven: User should be able to define toError and fromError functions.
2. Optional Parameters: Introduce replacer and reviver for added customization.
3. Application-Specific: Allow injectable rules for data filtering.
4. PK Conformance: Compatibility with existing Polykey workflows.

Tasks

• Parameterize toError and fromError: Accept user-defined implementations.
• Constructor Implementation: Include toError, fromError, replacer, and reviver in the constructor.
• Unit Testing: Create tests for the new features.
• Documentation: Update js-rpc documentation.

Acceptance Criteria

• User-defined toError and fromError functions can be successfully implemented.
• Optional replacer and reviver parameters function as expected.
• The system supports application-specific data filtering rules.
• Documentation is updated.

[CMCDragonkai]

This could be have been in #3.

[addievo]

[addievo]

Updated diagram, removing sensitiveReplacer, no longer required.

[CMCDragonkai]

A single replacer should be sufficient.

What does the API look like?

[addievo]

A single replacer should be sufficient.

What does the API look like?

RIght, sensitive replacer was related stack value of PK. So I will refactor that out.

API of?

[CMCDragonkai]

Give me an example of how it would be used here.

[addievo]

JSON RPC reponse :

{
"jsonrpc": "2.0", 
"error": {
    "code": -32700,
     "message": "Parse error",
     "data?": "Some data",
     "type": ErrorRPCParse,
     }, 
"id": null
}

fromError should return JSONValue object.

replacer is now a parameter too, which works on JSONValue passed by fromError.

replacer should be augemented to be able to extract error from JSON,
the application replacer should work on the library replacer,
since replacer acts on data within an error, i.e. on

"error": {
    "code": -32700,
     "message": "Parse error",
     "data?": "Some data",
     "type": ErrorRPCParse,
     }, 

toError does not necessarily need a reviver as a parameter.

[addievo]

fromError

1. It should now return a JSONValue object instead of a serialized JSON string. This makes it more flexible and allows for further manipulation if needed.

Updated fromError Function - draft

type JSONValue = string | number | boolean | null | { [key: string]: JSONValue } | JSONValue[];
function fromError(error: ErrorRPC<any>, sensitive: boolean = false): JSONValue {
  return {
    type: error.name,
    data: {
      message: error.message,
      code: error.code,
      description: error.description,
    },
  };
}

replacer

1. It should work on the JSONValue object returned by fromError.
2. It should be able to extract data from the error JSON object.

Updated replacer Function - draft

function replacer(key: string, value: any): any {
  if (value instanceof ErrorRPC) {
    return {
      code: value.code,
      message: value.description, 
      "data?": value.someData, 
      type: value.constructor.name, 
    };
  }
  return value;
}

toError

1. Should not necessarily need a reviver as a parameter.

Updated toError Function

function toError(errorData: any, metadata?: JSONValue): Error {
  const error = new Error(); // or any custom error class
  error.message = errorData.message || '';
  // Other custom operations can be performed here
  return error;
}

[CMCDragonkai]

The replacer function might need to be a bit more sophisticated. The replacer actually runs within the error property of the top level JSON object. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#the_replacer_parameter

You'll need to maintain a sort of context to know if you are under the tree of error property. It's basically a fragment cursor iterating over the tree (graph). So you just don't want to run it on anything other than the error property.

Given that code and message are mandatory properties... you can apply it to any property under error except code and message I reckon.

[CMCDragonkai]

Reopening this because RPCServer.createRPCServer looks incorrect.

[CMCDragonkai]

Errors can go both directions. Server to client and client to server. This means both RPCClient and RPCServerneedsfromErrorandtoError`.

On top of this, sensitive doesn't make sense to me. The existence of a specialised replacer function is basically intended to act as the sensitive remover. Why would there be a separate sensitive boolean? There'd be no use for this at all.

Furthermore // 1 minute is wrong. Remove spurious comments like this.

[CMCDragonkai]

Another issue is the idea of the default middleware. Is the default middleware necessary? If so, you cannot expect the user to supply middleware combined with the default if they are not aware of it.

You must then instead take in additional middleware and compose with the default middleware.

I'm not sure about this but @tegefaulkes can explain more later about this.

[CMCDragonkai]

Therefore the RPCServer API should look like this (not including the middleware parameters atm):

await RPCServer.createRPCServer({
  manifest,
  fromError,
  toError,
  filterSensitive
});

[tegefaulkes]

Default middleware is a compromise. We need to map the stream from UInt8Array to the JSONRPCMessages. If we wanted to allow the user to supply middleware that could work on the UInt8Array part of the stream such as applying compression/encryption. Then we needed to expose that to them. But the RPC requires some mapping from the raw stream to the RPC messages. This is the default middleware's job.

The alternative was to split up the user supplied middleware into stages. raw data stage and JSON message stage. To do this it will take a little bit of refactoring and prototyping with how the middleware is supplied and composed.

[CMCDragonkai]

Let's examine the relationship between JSON RPC errors and stream errors.

On the transport layer we have stream error codes. This is why we have codeToReason and reasonToCode functions. They are used by both sides of the duplex stream to deal convert a stream error code to a reason any type. The reason can be an any type, because this is accepted by readable.cancel(reason) and writable.abort(reason). This is what we call "transport-level stream errors". (There's also transport-level connection errors, but that's a separate thing).

On the application layer, we have RPC errors. In this case an error is encoded as per the JSON RPC protocol. https://www.jsonrpc.org/specification#error_object

Which might look something like this:

{"jsonrpc": "2.0", "error": {"code": -32601, "message": "Method not found"}, "id": "1"}

Therefore let's consider how toError and fromError works.

Server Side

The idea here is that during the handling of a stream. The handler function may throw up an error. Let's consider just for unary handlers.

handler() {
  throw new SpecialError('oh no');
}

In this case the fromError is applied to transform SpecialError to a specific serialisation of that SpecialError that should look like:

{"code": -32601, "message": "Method not found"}

This could be done by default to be:

(e: Error) => {
  return {
    code: e.code ?? utils.rpcDefaultCode,
    message: e.message
  };
};

The result is then combined with the rest of the JSON RPC message to eventually produce:

{
  "jsonrpc": "2.0", 
  "error": {
    "code": -32000, 
    "message": "oh no"
  }, 
  "id": null
}

On the other side, when it receives this JSON RPC error message. The parser/middleware is supposed to understand it, and thus convert it back to the exception object, that being of toError. The default of which is simply:

({ code, message }) => {
  return new Error(`${code.toString()} ${message.toString()}`),
};

This is actually similar to the default we have in QUICStream for codeToReason. However there's some discussion to change this to a ErrorQUICStreamReadable or ErrorQUICStreamWritable sort of thing for generic errors on the stream.

For the client, it now as per calling the networked handler, should throw up that exception object.

try {
  await rpcClient.doThisSpecialThing();
} catch (e) {
  console.log(e.message); // -32000 oh no
}

Client

For the client the opposite order occurs. But it still requires toError and fromError the same.

For Streams

Streams needs to do the same thing. Stream handlers have to now deal with errors being thrown into the async iterator/generator and dealt with too.

[CMCDragonkai]

Some discussion with @tegefaulkes.

• toError and fromError needs to be on both client and server.
• For unary and server streaming, the client side would not use fromError and the server side would not use toError because after sending the initial message, you cannot send any more messages from the client to server. You would just do p.cancel which can only result in a transport-level stream error code.
• For client streaming and duplex streaming, the client side would be using fromError and the server side would be using toError too, because they can actually throw an error into the stream.

Regardless, when creating RPCClient and RPCServer, you must able to inject both into them, because they may be utilised depending on the type of the handlers involved.

[CMCDragonkai]

Default middleware is a compromise. We need to map the stream from UInt8Array to the JSONRPCMessages. If we wanted to allow the user to supply middleware that could work on the UInt8Array part of the stream such as applying compression/encryption. Then we needed to expose that to them. But the RPC requires some mapping from the raw stream to the RPC messages. This is the default middleware's job.

The alternative was to split up the user supplied middleware into stages. raw data stage and JSON message stage. To do this it will take a little bit of refactoring and prototyping with how the middleware is supplied and composed.

So for now, if the user wants to supply custom middleware, they have to combine it with the default middleware explicitly? Is this guaranteed by the types?

[tegefaulkes]

Using the default middleware isn't enforced by types. But that mapping of Uint8Array -> JSONRPCMessages is.

[tegefaulkes]

Seems that errors sent through the forward path just wasn't supported and I think I know why.

The JSONRPC spec doesn't really allow it. The Error message is a response type message. So it's not really allowed

/**
 * This is the JSON RPC Request object. It can be a request message or
 * notification.
 */
type JSONRPCRequest<T extends JSONValue = JSONValue> =
  | JSONRPCRequestMessage<T>
  | JSONRPCRequestNotification<T>;

/**
 * This is a JSON RPC response object. It can be a response result or error.
 */
type JSONRPCResponse<T extends JSONValue = JSONValue> =
  | JSONRPCResponseResult<T>
  | JSONRPCResponseError;

I'm not sure it's a simple change to allow RPC errors on the forward path and even if we add it its not really a feature we need.

[addievo]

Therefore the RPCServer API should look like this (not including the middleware parameters atm):

await RPCServer.createRPCServer({
  manifest,
  fromError,
  toError,
  filterSensitive
});

whats filter sensitive for? The replacer can take in any key as a parameter, definable by the user

[addievo]

@CMCDragonkai, client doesn't currently transmit any errors to server, so whats exactly the need for fromError in client and conversely toError in server?

[addievo]

Moved to #18