buildo / metarpheus-js-http-api

DEPRECATED use https://github.com/buildo/metarpheus-io-ts instead

metarpheus-js-http-api

metarpheus-based js HTTP API

metarpheus-js-http-api's Issues

do we need/use the `.fromAPI` convention?

description

On a few old projects we had .fromApi defined on MyStruct.prototype for complex objects to be parsed from api. I think this convention is dead now, so we should remove this check

// TODO(gio): is `.fromAPI` used anywhere in our projects?

how to reproduce

• {optional: describe steps to reproduce defect}

specs

{optional: describe a possible fix for this defect, if not obvious}

misc

{optional: other useful info}

passing api responses through `returnType`

description

For every api response we get, we are feeding it into returnType, no matter what the build NODE_ENV is.
Do we want this even in production? What are the implications?

// TODO(gio): this is not under __DEV switch
// - why? maybe because in general, instantiating structs could be needed even in prod
// - does it have perf impacts?

how to reproduce

• {optional: describe steps to reproduce defect}

specs

{optional: describe a possible fix for this defect, if not obvious}

misc

{optional: other useful info}

js-minifier-friendly envify

description

be more gentle with Uglify and copy-paste the process.env.NODE_ENV !== 'production' switch instead of assigning to __DEV

how to reproduce

• {optional: describe steps to reproduce defect}

specs

{optional: describe a possible fix for this defect, if not obvious}

misc

{optional: other useful info}

`parser.test` should just return a boolean

description

as per title, the return value is not used in any way anymore

typecheck request body

requirements

possible now that metarpheus-tcomb produces it: buildo/metarpheus-tcomb#12

specs

add the feature in a backward compatible way

misc

{optional: other useful info}

avoid false positive with Structs as body type

t.assert checks for guard === true but MyStruct(value) is not a boolean :P

do we need to `stringifyParams`?

description

We are currently (implicitly) relying on the fact that whatever we are going to stringify already defines a correct toJSON and a correct toString.

This could be problematic since, for instance, it is true that Date.prototype.toJSON !== Date.prototype.toString, so we would stringify a date in some way if passed as part of JSON data (toJSON === toISOString) and in a different way if passed as query param (toString)

here are the two original TODOs:

// TODO(gio): do we really need this?
// stringifyParam = value => t.match(value,
//   t.Str, v => v,
//   t.Date, v => v.toISOString(),
//   t.Bool, v => String(v),
//   t.Number, v => String(v),
//   t.Nil, () => undefined, // undefined (query) params are stripped by axios down the road
//   t.Any, () => {
//     throw new Error('Unhandled param type', value);
//   }
// );

// const query = Object.keys(qq).reduce((ac, k) => {
//   return {
//     ...ac,
//     [k]: stringifyParam(qq[k])
//   };
// }, {});

handle routeParamType shape change from https://github.com/buildo/metarpheus-tcomb/pull/25

no way to add parsers without redefining default ones

description

{describe the defective behavior}

how to reproduce

• {optional: describe steps to reproduce defect}

specs

{optional: describe a possible fix for this defect, if not obvious}

misc

{optional: other useful info}

`npm test` hangs

warn if api responds with non-gzipped content

requirements

as per title, add a nice warning if this happens

specs

• there's already a wip (skipped) test
• TODO

misc

{optional: other useful info}

content-type: application/json

description

an old (from [email protected] era) TODO said:

// TODO(gio): not sure why but the automatically inferred Content-Type
// is omitted if providing a custom transformRequest
if (['post', 'put', 'patch'].indexOf(method) !== -1) {
  headers['Content-Type'] = 'application/json';
}

do we still need this with the current axios version?

nice to have: pass data={} in case of POST with no body params

requirements

Currently, in case of POST requests, wiro metarpheus apis require an empty json body ({}) even if there are no body params in the request. The developer is thus required to call the generated api method as methodName({ data: {} }) even if there's no data to pass.

To be kinder and for consistency, we could pass this empty body automatically

specs

• default data to {} if method is POST
• probably the same applies to PUT requests too?

misc

• double check what happens for non-wiro APIs (e.g. Alinity PRO): is the API happy to receive the empty object body? (I think so)
  • not sure what to do if not :/

Be sure to always reject if anything in the response chain fails

See https://github.omnilab.our.buildo.io/buildo/aliniq/issues/4703
In this case, the promise was not properly rejected

• make sure to always reject
• add tests for this

warn when passing data (body) to an api but no metarpheus typing is available for it

requirements

as per title

specs

{optional: describe technical specs to implement this feature, if not obvious}

misc

{optional: other useful info}

be sure to reject if error response parsing fails

description

similar to #11 , but specific to "error parsing" only

how to reproduce

discovered in https://github.omnilab.our.buildo.io/buildo/aliniq/issues/4721

specs

try/catch error parsing and always return something well-formed

misc

{optional: other useful info}

default `parseError` fn doesn't work

description

as per title (tested on qia)
it returns an undefined status, while we typically expect an HTTP status there

how to reproduce

• on qia, try any call while network is unreachable

specs

• probably due to the axios upgrade (the same function was working with [email protected])
• rewrite it to be compatible

misc

• another test to do (other than network unreachable): on qia, try a "create new user" using an already existing user name (it should correctly parse a standard 422)

Use scriptoni for linting