SAP / karma-ui5

A Karma plugin for UI5
Apache License 2.0
69 stars 26 forks source link

UI5 Tooling Middleware behaves differently through `karma-ui5` - request body not readable #410

Open LukasHeimann opened 2 years ago

LukasHeimann commented 2 years ago

In UI5 Tooling, I can create a custom middleware: https://sap.github.io/ui5-tooling/pages/extensibility/CustomServerMiddleware/

The documentation states:

For this you can plug custom middleware implementations into the internal express server of the UI5 Server module.

This is nice, as, with express, you can use res.send(anyJSON) and it automatically takes care of the headers.

In karma-ui5, the middlewares of UI5 Tooling work out of the box, which is really nice, but it seems like express-based methods are missing from req and res. A middleware using res.send() works with ui5 serve, but fails within karma -- it needs to be replaced with res.setHeader('Content-Type', 'application/json'); res.end(JSON.stringify(...));.

Is this expected? Can improvements be made in karma-ui5 so that UI5 middleware behavior doesn't change when run through karma? Or am I missing something?

LukasHeimann commented 2 years ago

I have found a way to rectify some of the problems by doing what express internally does:

const express = require('express');

let app;
function getApp() {
  if (!app) app = express();
  return app;
}

function enhance(req, res, next) {
  if (Object.getPrototypeOf(req) !== express.request) {
    // called from karma, request and response are not enhanced with express methods
    // this is basically what express does as well: https://github.com/expressjs/express/blob/master/lib/middleware/init.js
    req.app = getApp();
    res.app = getApp();
    req.res = res;
    res.req = req;
    req.next = next;
    Object.setPrototypeOf(req, express.request);
    Object.setPrototypeOf(res, express.response);
    res.locals = res.locals || Object.create(null);
  }
}

function makeRouter() {
  const router = express.Router();
  router.use(enhance);
  return router;
}

However, I can't access the request body in my Middleware. Somehow, the request stream is already consumed (req.readable === false) in my middleware. What is happening here? Please get into contact.

LukasHeimann commented 2 years ago

For debugging, I added a couple of checks here: https://github.com/SAP/karma-ui5/blob/master/lib/framework.js#L531

The stream seems to be paused, but still readable at this point in the middleware chain. In my UI5 middleware, the stream was neither paused nor readable. I've added:

    const orig = req.resume;
    req.resume = (...args) => {
      console.log(new Error("resumed"))
      orig.apply(req, args)
    }

I've found that node itself seems to flush the stream at some point before my middleware, which I found not helpful. I've then replaced the router dependency in UI5 server with express.Router, as router had some async logic (https://github.com/pillarjs/router/blob/master/lib/route.js#L136) and express.Router didn't have that (compare also https://github.com/SAP/ui5-tooling/issues/606).

At that point, the stream was resumed by karma source files here: https://github.com/karma-runner/karma/blob/master/lib/middleware/source_files.js#L58 It didn't yet make the stream readable in my UI5 middleware, but removing the resume in the source_files did. I'm not sure, why, though.

LukasHeimann commented 2 years ago

(The stream not being readable caused a problem within the body-parser package, which will hopefully be fixed after https://github.com/stream-utils/raw-body/pull/58 lands)

matz3 commented 2 years ago

This seems to be related to https://github.com/SAP/karma-ui5/issues/344#issuecomment-937551659, right?

LukasHeimann commented 2 years ago

Ah, yes, the remaining issue in fact does, I will watch that one as well!

However, it would also be great if karma-ui5 would already enrich request and response with the express methods that are available in UI5 tooling, but not through karma (as it only uses connect).

Potentially, this would just be an additional middleware as outlined here: https://github.com/SAP/karma-ui5/issues/410#issuecomment-1072227181

matz3 commented 2 years ago

Right now we don't see this as an improvement to be made directly into UI5 Tooling or karma-ui5. If your middleware is using some express APIs, you could do the mentioned enhancement within your middleware or via a separate custom middleware.

The other issue related to reading the request body will be tracked via #344.

LukasHeimann commented 2 years ago

@matz3 please reopen this as documentation bug.

https://sap.github.io/ui5-tooling/pages/extensibility/CustomServerMiddleware/ clearly states that the middleware will be plugged into an express server, implying the availability of express methods on request and response. It also explicitly links to the express API documentation.

As stated by you, this is simply not true.