Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
LukasHeimann opened this issue Mar 7, 2022 · 7 comments

Comments

@LukasHeimann
Copy link

LukasHeimann commented Mar 7, 2022

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
Copy link
Author

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
Copy link
Author

LukasHeimann commented Mar 21, 2022

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 SAP/ui5-tooling#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
Copy link
Author

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

@LukasHeimann LukasHeimann changed the title UI5 Tooling Middleware behaves differently through karma-ui5 UI5 Tooling Middleware behaves differently through karma-ui5 - request body not readable Mar 21, 2022
@matz3
Copy link
Member

matz3 commented Mar 21, 2022

This seems to be related to #344 (comment), right?

@LukasHeimann
Copy link
Author

LukasHeimann commented Mar 21, 2022

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: #410 (comment)

@matz3 matz3 self-assigned this Mar 28, 2022
@matz3
Copy link
Member

matz3 commented Apr 14, 2022

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.

@matz3 matz3 closed this as completed Apr 14, 2022
@matz3 matz3 removed their assignment Apr 14, 2022
@LukasHeimann
Copy link
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants