oc icon indicating copy to clipboard operation
oc copied to clipboard

OC doesn't show proper message when publishing fails

Open ashmeet-kandhari opened this issue 2 years ago • 2 comments

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 });
                 }
             }
         }


ashmeet-kandhari avatar Mar 06 '23 10:03 ashmeet-kandhari

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

ricardo-devis-agullo avatar Mar 07 '23 09:03 ricardo-devis-agullo

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

ashmeet-kandhari avatar Mar 08 '23 11:03 ashmeet-kandhari