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

Return error response in submitBom #1108

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Return error response in submitBom #1108

merged 1 commit into from
Jun 11, 2024

Conversation

marob
Copy link
Contributor

@marob marob commented May 23, 2024

When an error occurs, there is no possibility for the calling code to know the details.

In my case, I'd like to be able to log error response body, in particular to be able to know which validation is KO when uploading a SBOM to DependencyTrack (that recently enabled JsonSchema validation on upload).

@marob marob requested review from prabhu and setchy as code owners May 23, 2024 17:45
index.js Outdated Show resolved Hide resolved
@marob marob force-pushed the patch-1 branch 2 times, most recently from 255067a to 87d8f9b Compare May 23, 2024 19:52
@prabhu
Copy link
Contributor

prabhu commented May 23, 2024

@marob Can we also enhance server.js so that it returns the error message in line 160

https://github.com/marob/cdxgen/blob/87d8f9bf2c1036341d47775998931786ff9cf788/server.js#L160

      res.writeHead(500, { "Content-Type": "application/json" });
      return res.end(
        "{'error': 'true', 'message': 'simpler error message'}\n",
      );

@marob
Copy link
Contributor Author

marob commented May 23, 2024

@marob Can we also enhance server.js so that it returns the error message in line 160

https://github.com/marob/cdxgen/blob/87d8f9bf2c1036341d47775998931786ff9cf788/server.js#L160

      res.writeHead(500, { "Content-Type": "application/json" });
      return res.end(
        "{'error': 'true', 'message': 'simpler error message'}\n",
      );

OK. I'll do it next week.

But I'm questioning myself: shouldn't we throw an error instead of returning it like I implemented in my PR?

@prabhu
Copy link
Contributor

prabhu commented May 24, 2024

@marob may be use the options.failOnError to determine that?

@prabhu
Copy link
Contributor

prabhu commented May 28, 2024

@marob, I have a large PR coming up in #1119. Is it possible to wait and rebase once it is merged?

@marob
Copy link
Contributor Author

marob commented May 30, 2024

OK @prabhu. I've rebased and applied the required modifications on server.js and cdxgen.js

@setchy
Copy link
Member

setchy commented May 30, 2024

@marob - awesome!

Would you be able to update the recently added openapi spec - https://github.com/CycloneDX/cdxgen/blob/master/lib/server/openapi.yaml - to have the new response code and response structure. Happy to help if you'd like, just lmk

@marob
Copy link
Contributor Author

marob commented May 30, 2024

@setchy I've added the requested modification in openapi.
I've also changed the structure of the response for errors. I hope it's OK.
I'm wondering if we should keep an array of string as details for the error as it may be very specific to my case. Maybe we could stringify my array of string and put it directly at the end of the error?

index.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
@prabhu
Copy link
Contributor

prabhu commented Jun 11, 2024

@setchy @marob what is left on this one? May be we have to rebase again due to some recent merges (apologies)

@setchy
Copy link
Member

setchy commented Jun 11, 2024

@setchy @marob what is left on this one? May be we have to rebase again due to some recent merges (apologies)

looks good to me 👍

When an error occurs, there is no possibility for the calling code to know the details.

In my case, I'd like to be able to log error response body, in particular to be able to know which validation is KO when uploading a SBOM to DependencyTrack (that recently enabled JsonSchema validation on upload).

Signed-off-by: Maxime Robert <robert.maxime@gmail.com>
Signed-off-by: Maxime Robert <maxime.robert@smile.fr>
@prabhu prabhu merged commit 8e30af0 into CycloneDX:master Jun 11, 2024
20 checks passed
@marob marob deleted the patch-1 branch June 12, 2024 14:30
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

3 participants