fusionjs / fusion-plugin-rpc

Migrated to https://github.com/fusionjs/fusionjs

FUSION.JS

Modern framework for fast, powerful React apps

What is it?

fu·sion — noun

The process or result of joining two or more things together to form a single entity.

Fusion.js, Uber’s open source universal web framework, represents the fusion of the client and the server. It's geared for server-side rendering out of the box, and its plugin-driven architecture allows for complex frontend and backend logic to be encapsulated in a single plugin:

import App from 'fusion-react';
import Router from 'fusion-plugin-react-router';

export default () => {
  const app = new App(<div>...</div>);

  /*
  One line of code sets up everything you need for routing:
  - Server rendering
  - React Providers on both server and browser
  - Bundle splitting integration
  - Hot module reloading support
  */
  app.register(Router);

  return app;
}

We initially built Fusion.js to make our own websites easier to maintain, but were so impressed with the benefits that we decided to offer it to the community as an open source project!

Try it out

If you're interested in giving Fusion.js a shot, Overview and Core Concepts are great places to start.

Contributing

This is a monorepo of all open source Fusion.js packages maintained using Yarn v2. Take a look at CONTRIBUTING.md for info on how to develop in this repo.

License

MIT

fusion-plugin-rpc's Issues

Add support for FormData

Rationale: Currently, RPC always assume JSON data. Supporting FormData would enable the ability to do file uploads

Add initial implementation

Add Flow types to source files

Problem/Rationale

Source code currently lacks Flow types. As a widely used plugin, it could benefit from static checking.

Solution/Change/Deliverable

Implement Flow types

Add MIT license header to top of all source files

Ensure plugin provides `{from(ctx)}`

Plugins developed by the FusionJS team should provide a consistent interface for obtaining a request-scoped service instance, as described here: fusionjs/fusion-core#72 (comment)

Rationale:

A consistent interface reduces the cognitive load on framework users since it becomes one less thing that requires looking up documentation

Deliverable:

Change the provided interface from (ctx) => T to {from: (ctx) => T}

Migrate fusion-plugin-rpc to createPlugin

Thoughts on always sending 200 status code?

HTTP status codes don't translate nicely to the web rpc pattern. We currently send a 400 status code on RPC failure, however this doesn't really make any sense. From the RPC framework perspective, the request succeeded. We could potentially send some error status code for serialization / deserialization errors, but apart from that I think we should simply default to a 200 status code, and allow the user to override it via the ctx argument passed to the rpc handler.

Thoughts?

Update fusion-plugin-rpc handling of routePrefix to not prepend to fetch calls

Type of issue

Bug

Description

fusion-plugin-rpc should integrate better with routePrefix. The standard way of handling routePrefix is to provide a fetch implementation that automatically appends the prefix.

Current behavior

fusion-plugin-rpc prepends the routePrefix to its calls to fetch, which results in a double route prefix url.

Expected behavior

fusion-plugin-rpc should call fetch without prepending the route prefix.

Steps to reproduce

1. run a fusion application with the ROUTE_PREFIX environment variable set
2. add client side rpc data fetching

Update dependencies

Update `README.md` to `createPlugin` API

Problem/Rationale

Parts of the readme remain out of date.

Solution/Change/Deliverable

Update README.md

Use Buildkite and Docker for CI

Update API to use .request rather than copying methods

We currently copy methods from {handlers} onto the RPC service. In the browser, we send down a serialized version of those handlers. While this has a nice API, it has a few problems.

1. Unnecessary additions to html size
2. Bad developer experience when using hot-reloading. If you add a new handler on the server, you need to refresh to page in order to use it.

A .request API is simpler and solves these problems.

Add mock export for use in testing

It can be useful to mock handlers when writing unit tests, both in the browser and server.

Import Flow declaration for 'fetch'

Problem/Rationale

Codebase currently has a copy/pasta type definition for fetch. Other plugins also have this same copy/pasta.

Solution/Change/Deliverable

We can either:

• Centralize the type definition in a common FusionJS package.
• Find common Flow definition for fetch and import the libdef.

Update dependencies

Migrate fusion-plugin-rpc to DI

Update handler api to take `ctx` as a second argument

The handler API should take ctx as a second argument. For example:

app.plugin(RPC, {handlers: {getUser: (args, ctx) => ''}});

Make createPlugin asynchronous

Problem/Rationale

Instantiation of a plugin may not always be best served by being synchronous.

Solution/Change/Deliverable

Allow createPlugin and it's associated methods to be asynchronous.

Test Issue

Delete after 2018-02-20

Add timing events

See TODO in server.js

Fix flow types

Action Required: Fix Renovate Configuration

There is an error with this repository's Renovate configuration that needs to be fixed. As a precaution, Renovate will stop PRs until it is resolved.

Error type: Preset name not found within published preset config (monorepo:angularmaterial). Note: this is a nested preset so please contact the preset author if you are unable to fix it yourself.

Add Token dependencies to readme

Problem/Rationale

Documentation regarding Fusion API is out of date given recent changes to leverage new Dependency Injection architecture.

Solution/Change/Deliverable

Update documentation

Migrate to new fusion-tokens api

Update error message to have more info

Unhelpful error message.

fusion-plugin-rpc/src/server.js

Lines 150 to 152 in 72c38fb

	: new Error(
	'UnknownError - Use ResponseError for more detailed error messages'
	);

Type of issue

Feature request

Description

At the moment, the aforementioned error message is unclear. Adding a little more info won't hurt.
The first thought that came to my mind when I saw the message was that I would need to extract ResponseError from the original thrown error message.

Current behavior

message: "UnknownError - Use ResponseError for more detailed error messages"

Expected behavior

message: "UnknownError - Use ResponseError from fusion-plugin-rpc (or fusion-plugin-rpc-redux-react if you are using React) package for more detailed error messages"

Steps to reproduce

1. Just throw an error in your rpc call and you'll see the error message

Your environment

• fusion-plugin-rpc version: 2.3.0

• Node.js version (node --version): 8.11.3

• npm version (npm --version): 5.6.0

• Operating System: macOS 10.14.3

Update API for Error Handling

The current API of rpc handlers has users return a Promise that resolves to a response for the rpc request. If the promise rejects, the plugin will use the properties on the error (other than the stack trace) as the response. This causes issues however when a handler allows an error to bubble from a library. The bubbled error will be used for the response, and can contain information the user may not want in the response, such as leaked information from the request.

To resolve this, we should enforce that endpoints have explicit error handling that describes exactly the data they want to respond with. We can do this by ignoring the data in rejection errors unless they come from a special error subclass ResponseError which will be provided by the rpc plugin.

Example RPC Handler

import {ResponseError} from 'fusion-plugin-rpc';

function testHandler(args, ctx) {
   try {
    doThing();
  } catch (e) {
    throw new ResponseError('Failed to do thing', { custom: 'metadata' });
  }
}
// Responds with
{
  status: 'failure'
  data: {
    message: 'Failed to do thing',
    meta: {
      custom: 'metadata',
    },
  }
}

If a normal error is thrown, the plugin will log a warning and respond with an unknown error message.

Remove unnecessary handlers token from browser

We don't need to have special code ensuring rpc handlers are
not registered in the browser since we don't allow for extraneous
config registration.

Investigate ways to improve rpc error handling

The current process for failing an RPC is to return a Promise that rejects. This works fairly well, however it falls a bit short when you want to serialize data with the RPC failure.

It is generally good practice to only instances of an Error object to Promise.reject(), and similarly to only throw Error objects (see https://eslint.org/docs/rules/prefer-promise-reject-errors and https://eslint.org/docs/rules/no-throw-literal). However, lets say you want to signal an RPC failure, and you want to serialize some JSON down to the client. We currently only send the error.message data to the client, meaning you are very restricted in how you handle errors.

Node has somewhat recently standardized on using Error.code to signal the type of error (see https://nodejs.org/api/errors.html#errors_error_code). A good first step would be supporting this property on the rejected error.

If error.code is not enough (which I suspect it won't be), we could support serializing arbitrary JSON on the error.meta key.

Thoughts?

Update check for ctx in constructor

We need to update the core plugin api to pass null or undefined to the Service constructor when a user calls .of(). Once that is done, we can update our check to check for truthiness rather than ctx.headers.