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

Allow custom status code when returning a stream #1343

Merged
merged 1 commit into from
Nov 24, 2022
Merged

Allow custom status code when returning a stream #1343

merged 1 commit into from
Nov 24, 2022

Conversation

NicholasPeretti
Copy link
Contributor

@NicholasPeretti NicholasPeretti commented Nov 17, 2022

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you written unit tests?
  • Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)?
  • This PR is associated with an existing issue?

Closing issues

Put closes #XXXX (where XXXX is the issue number) in your comment to auto-close the issue that your PR fixes.

If this is a new feature submission:

  • Has the issue had a maintainer respond to the issue and clarify that the feature is something that aligns with the goals and philosophy of the project?

Potential Problems With The Approach

Test plan

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello there NicholasPeretti 👋

Thank you and congrats 🎉 for opening your first PR on this project.✨

We will review the following PR soon! 👀

@NicholasPeretti
Copy link
Contributor Author

Hello there 👋

I've noticed that when I return a readable stream in my container the status code is being ignored.
I've tested this code in my own application and it works perfectly.

However, I wasn't able to properly install the dependencies of the codebase and run unit tests on my laptop 🤔
I see in you package.json that you need Node 12.0.0 or higher.

I first tried with Node 18, and then with Node 12, but I couldn't even install the dependencies 🤔
Any help?

@NicholasPeretti
Copy link
Contributor Author

Just in case someone else is affected, I wrote this little script to fix the problem.
It works with Express only.

Please adapt it accordingly to your project.
You can then edit your build script as tsoa spec-and-routes -c src/tsoa/tsoa.json && node patch-tsoa.js

import fs from "fs";
import path from "path";
import tsoaConfig from "./tsoa.json";

const TSOA_GENERATE_ROUTES_FILE_PATH = path.resolve(
  process.cwd(),
  tsoaConfig?.routes?.routesDir,
  "routes.ts"
);

(async () => {
  fs.readFile(TSOA_GENERATE_ROUTES_FILE_PATH, "utf-8", (err, data) => {
    if (err) {
      console.error("Cannot open the Tsoa generated routes", err);
      return;
    }

    const updatedCode = data.replace(
      "data.pipe(response);",
      "response.status(statusCode || 200);data.pipe(response);"
    );

    fs.writeFile(TSOA_GENERATE_ROUTES_FILE_PATH, updatedCode, (err) => {
      if (err) {
        console.error("Cannot apply Tsoa patch", err);
        return;
      }

      console.log("Tsoa patch applied successfully");
    });
  });
})();

@WoH
Copy link
Collaborator

WoH commented Nov 21, 2022

Would you mind adding an integration test here? At least for express?

@WoH WoH merged commit be29e09 into lukeautry:master Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants