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

OC doesn't show proper message when publishing fails #1326

Open
ashmeet-kandhari opened this issue Mar 6, 2023 · 2 comments
Open

OC doesn't show proper message when publishing fails #1326

ashmeet-kandhari opened this issue Mar 6, 2023 · 2 comments

Comments

@ashmeet-kandhari
Copy link
Contributor

Who is the bug affecting?

  • component creators

What is affected by this bug?

  • OC publishing client api

When does this occur?

  • Most of the times when components fail during publishing.

Where on the platform does it happen?

  • Linux

How do we replicate the issue?

Expected behavior (i.e. solution)

  • If oc publishing fails, it should show us proper error message saying why it has failed instead of empty error message
An error happened when publishing the component: {}

What version of OC, Node.js and OS are you using?

Other Comments

  • Proposed solution is to stringify the error object instead of extracting message field which can change the name (err.msg or err.message).
  • Sample patch file
--- ./oc-patch/oc-registry-routes/publish_original.js	2023-03-01 11:37:29.000000000 +0000
+++ ./oc-patch/oc-registry-routes/publish_modified.js	2023-03-01 11:40:50.000000000 +0000
@@ -70,25 +70,35 @@
                 res.status(200).json({ ok: true });
             }
             catch (err) {
+                console.error('Patched OC->Registry->Routes->Publish: ', JSON.stringify(err));
+                let errorMessage;
+                if(err.msg) {
+                    errorMessage = err.msg;
+                } else if (err.message){
+                    errorMessage = err.message;
+                } else {
+                    errorMessage = JSON.stringify(err);
+                }
+
                 if (err.code === 'not_allowed') {
-                    res.errorDetails = `Publish not allowed: ${err.msg}`;
-                    res.status(403).json({ error: err.msg });
+                    res.errorDetails = `Publish not allowed: ${errorMessage}`;
+                    res.status(403).json({ error: err });
                 }
                 else if (err.code === 'already_exists') {
-                    res.errorDetails = `Component already exists: ${err.msg}`;
-                    res.status(403).json({ error: err.msg });
+                    res.errorDetails = `Component already exists: ${errorMessage}`;
+                    res.status(403).json({ error: err });
                 }
                 else if (err.code === 'name_not_valid') {
-                    res.errorDetails = `Component name not valid: ${err.msg}`;
-                    res.status(409).json({ error: err.msg });
+                    res.errorDetails = `Component name not valid: ${errorMessage}`;
+                    res.status(409).json({ error: err });
                 }
                 else if (err.code === 'version_not_valid') {
-                    res.errorDetails = `Component version not valid: ${err.msg}`;
-                    res.status(409).json({ error: err.msg });
+                    res.errorDetails = `Component version not valid: ${errorMessage}`;
+                    res.status(409).json({ error: err });
                 }
                 else {
-                    res.errorDetails = `Publish failed: ${err.msg}`;
-                    res.status(500).json({ error: err.msg });
+                    res.errorDetails = `Publish failed: ${errorMessage}`;
+                    res.status(500).json({ error: err });
                 }
             }
         }


@ricardo-devis-agullo
Copy link
Collaborator

The whole treating of errors should be normalized honestly across the codebase, but I'm fine with this adhoc solution for the moment

@ashmeet-kandhari
Copy link
Contributor Author

Thanks @ricardo-devis-agullo ,
Raised PR for review

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

No branches or pull requests

2 participants