fix: improve error message for generic openid-client error MONGOSH-2487 (#230)

* fix: improve error message for generic openid-client error MONGOSH-2487

Similar to previous commits, this change ensures that diagnostic information
is not lost through recent changes to openid-client's error system, where
currently a number of errors are grouped together under "something went wrong"
(https://github.com/panva/openid-client/blob/a1e4a4fd88fcdff12d88e3ff73f3e27fe5df4252/src/index.ts#L975)
and the library expects consumers to consistently expose the `cause` property
of the error object.

This also unifies the two existing `messageFromError` and `errorString`
implementations, which happened to duplicate each other.

* fixup: electron compat

* fixup: do not add "[object Response]" to errors

* fixup: add anchors to regex

Author: addaleax

src/plugin.spec.ts

@@ -40,6 +40,8 @@ import type {
   TokenMetadata,
 } from '@mongodb-js/oidc-mock-provider';
 import { OIDCMockProvider } from '@mongodb-js/oidc-mock-provider';
+import type { Server as HTTPSServer } from 'https';
+import { createServer as createHTTPSServer } from 'https';
 
 // node-fetch@3 is ESM-only...
 // eslint-disable-next-line @typescript-eslint/consistent-type-imports
@@ -1417,6 +1419,36 @@ describe('OIDC plugin (mock OIDC provider)', function () {
   });
 
   context('HTTP request handling', function () {
+    let badHTTPSServer: HTTPSServer;
+
+    before(async function () {
+      badHTTPSServer = createHTTPSServer(
+        {
+          key: await fs.readFile(
+            // https://github.com/nodejs/node/blob/becb55aac3f7eb93b03223744c35c6194f11e3e9/test/fixtures/keys/agent2-key.pem
+            path.resolve(__dirname, '..', 'test', 'self-signed-key.pem')
+          ),
+          cert: await fs.readFile(
+            // https://github.com/nodejs/node/blob/becb55aac3f7eb93b03223744c35c6194f11e3e9/test/fixtures/keys/agent2-cert.pem
+            path.resolve(__dirname, '..', 'test', 'self-signed-cert.pem')
+          ),
+        },
+        (req, res) => {
+          res.writeHead(200, { 'Content-Type': 'application/json' });
+          res.end(JSON.stringify({ message: 'Hello World' }));
+        }
+      );
+      badHTTPSServer.listen(0, 'localhost');
+      await once(badHTTPSServer, 'listening');
+    });
+
+    after(async function () {
+      if (badHTTPSServer) {
+        badHTTPSServer.close();
+        await once(badHTTPSServer, 'close');
+      }
+    });
+
     it('will track all outgoing HTTP requests', async function () {
       const pluginHttpRequests: string[] = [];
       const localServerHttpRequests: string[] = [];
@@ -1555,7 +1587,7 @@ describe('OIDC plugin (mock OIDC provider)', function () {
       expect(customFetch).to.have.been.called;
     });
 
-    it('logs helpful error messages', async function () {
+    it('logs helpful error messages for HTTP failures', async function () {
       const outboundHTTPRequestsCompleted: any[] = [];
 
       getTokenPayload = () => Promise.reject(new Error('test failure'));
@@ -1595,6 +1627,48 @@ describe('OIDC plugin (mock OIDC provider)', function () {
       });
     });
 
+    it('logs helpful error messages for TLS failures', async function () {
+      const outboundHTTPRequestsFailed: any[] = [];
+      const port = (badHTTPSServer.address() as AddressInfo).port;
+
+      const plugin = createMongoDBOIDCPlugin({
+        openBrowserTimeout: 60_000,
+        openBrowser: fetchBrowser,
+        allowedFlows: ['auth-code'],
+        redirectURI: 'http://localhost:0/callback',
+      });
+
+      plugin.logger.on(
+        'mongodb-oidc-plugin:outbound-http-request-failed',
+        (ev) => outboundHTTPRequestsFailed.push(ev)
+      );
+
+      let selfSignedReason = 'self-signed certificate';
+      try {
+        await requestToken(plugin, {
+          issuer: `https://localhost:${port}/`,
+          clientId: 'mockclientid',
+          requestScopes: [],
+        });
+        expect.fail('missed exception');
+      } catch (err: any) {
+        // Electron and Node.js provide different error messages
+        selfSignedReason = err.message.includes('self-signed certificate')
+          ? 'self-signed certificate'
+          : 'self signed certificate';
+        expect(err.message).to.include(
+          `Unable to fetch issuer metadata for "https://localhost:${port}/": something went wrong (caused by: request to https://localhost:${port}/.well-known/openid-configuration failed, reason: ${selfSignedReason}`
+        );
+      }
+      const entry = outboundHTTPRequestsFailed.find(
+        (ev) =>
+          ev.url ===
+          `https://localhost:${port}/.well-known/openid-configuration`
+      );
+      expect(entry).to.exist;
+      expect(entry.error).to.include(selfSignedReason);
+    });
+
     it('handles JSON failure responses from the IDP', async function () {
       overrideRequestHandler = (url, req, res) => {
         if (new URL(url).pathname.endsWith('/token')) {

src/plugin.ts

@@ -11,7 +11,6 @@ import {
   errorString,
   getRefreshTokenId,
   improveHTTPResponseBasedError,
-  messageFromError,
   nodeFetchCompat,
   normalizeObject,
   throwIfAborted,
@@ -525,7 +524,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
       throw new MongoDBOIDCError(
         `Unable to fetch issuer metadata for ${JSON.stringify(
           serverMetadata.issuer
-        )}: ${messageFromError(err)}`,
+        )}: ${errorString(err)}`,
         {
           cause: err,
           codeName: 'IssuerMetadataDiscoveryFailed',
@@ -844,7 +843,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
                 browserHandle?.once('error', (err) =>
                   reject(
                     new MongoDBOIDCError(
-                      `Opening browser failed with '${messageFromError(
+                      `Opening browser failed with '${errorString(
                         err
                       )}'${extraErrorInfo()}`,
                       { cause: err, codeName: 'BrowserOpenFailedSpawnError' }

src/util.ts

@@ -47,12 +47,6 @@ export async function withAbortCheck<
   }
 }
 
-export function errorString(err: unknown): string {
-  return String(
-    typeof err === 'object' && err && 'message' in err ? err.message : err
-  );
-}
-
 // AbortSignal.timeout, but consistently .unref()ed
 export function timeoutSignal(ms: number): AbortSignal {
   const controller = new AbortController();
@@ -127,15 +121,26 @@ export function validateSecureHTTPUrl(
   }
 }
 
-export function messageFromError(err: unknown): string {
-  return String(
-    err &&
-      typeof err === 'object' &&
-      'message' in err &&
-      typeof err.message === 'string'
-      ? err.message
-      : err
-  );
+export function errorString(err: unknown): string {
+  if (
+    !err ||
+    typeof err !== 'object' ||
+    !('message' in err) ||
+    typeof err.message !== 'string'
+  ) {
+    return String(err);
+  }
+  const cause = getCause(err);
+  let { message } = err;
+  if (cause) {
+    const causeMessage = errorString(cause);
+    if (
+      !message.includes(causeMessage) &&
+      !causeMessage.match(/^\[object.+\]$/i)
+    )
+      message += ` (caused by: ${causeMessage})`;
+  }
+  return message;
 }
 
 const salt = randomBytes(16);

test/self-signed-cert.pem

@@ -0,0 +1,21 @@
+-----BEGIN CERTIFICATE-----
+MIIDgzCCAmsCFF3cqhUsBYLNh3bCVatENxZcoY7rMA0GCSqGSIb3DQEBCwUAMH0x
+CzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTELMAkGA1UEBwwCU0YxDzANBgNVBAoM
+BkpveWVudDEQMA4GA1UECwwHTm9kZS5qczEPMA0GA1UEAwwGYWdlbnQyMSAwHgYJ
+KoZIhvcNAQkBFhFyeUB0aW55Y2xvdWRzLm9yZzAgFw0yMjA5MDMxNDQ2NTFaGA8y
+Mjk2MDYxNzE0NDY1MVowfTELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQswCQYD
+VQQHDAJTRjEPMA0GA1UECgwGSm95ZW50MRAwDgYDVQQLDAdOb2RlLmpzMQ8wDQYD
+VQQDDAZhZ2VudDIxIDAeBgkqhkiG9w0BCQEWEXJ5QHRpbnljbG91ZHMub3JnMIIB
+IjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvqt/4KDehQLLDH+I2KXOxg4G
+WfNBISWmKlExPBfz9i1LY/rwoRwryv3Lpr40M05Dx+Rt4LMC+If7NGvrV8hdNSOz
+jW7P7R6upVdNXpeZDmHvhq+G/xv+x/Hdv3+Sdm/JC8TD2HRYcHSSWsirRbfA9eJe
+L0ADh1mJGNpWS+9FNXtbR3LRWsRwNjP1Lb39tXIsfHiWrJ/F6yAhWOU+ZZvvjazp
+bZX7Kes0lxVtyWCzLFpnzYa/gajGLdGJwTrfKXsSz2wk6szKlbO0mzX0aHviPRPT
+ftUVs91qORJ8tkAU4u78bpV0eCM8tVJh/N/oSm7ysLUjxhJrfNxHmmkGyaRL/QID
+AQABMA0GCSqGSIb3DQEBCwUAA4IBAQA+94z0pI0JEU1dX4bHGkhP6hwmv5tu7KlA
+R0hK33pF+boiagbySHrXW/y119VLp+o1FjuOlS4ETgAjcIjN2dDmJc0JEj6jnXyc
+4IYhRMDg4INAnmXX9bdCmpYuvhw/73cuxkdkMxH8p4O7v5HSqfpwjTEX8tWtpeMI
+IZ4+H/ddOKyvF3SO8lfrYJ7TXyypWfxzEiBuJnhZgpMG7zpZMGIzTkcN9VFTCv8d
+DCW0Lr2Ix/GY7nf/R9zDFnEZTW6IIkRp9UsUdOrgqgfSxp/C48foFv7gqMO/9PD8
+E8uE8986AFd5cK67imYPspHXv5UycySifwsSixi0hI9lDZqUIoWH
+-----END CERTIFICATE-----

test/self-signed-key.pem

@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEpAIBAAKCAQEAvqt/4KDehQLLDH+I2KXOxg4GWfNBISWmKlExPBfz9i1LY/rw
+oRwryv3Lpr40M05Dx+Rt4LMC+If7NGvrV8hdNSOzjW7P7R6upVdNXpeZDmHvhq+G
+/xv+x/Hdv3+Sdm/JC8TD2HRYcHSSWsirRbfA9eJeL0ADh1mJGNpWS+9FNXtbR3LR
+WsRwNjP1Lb39tXIsfHiWrJ/F6yAhWOU+ZZvvjazpbZX7Kes0lxVtyWCzLFpnzYa/
+gajGLdGJwTrfKXsSz2wk6szKlbO0mzX0aHviPRPTftUVs91qORJ8tkAU4u78bpV0
+eCM8tVJh/N/oSm7ysLUjxhJrfNxHmmkGyaRL/QIDAQABAoIBAQCPnrT7KZGTVTBH
+IMWekv52ltfX53BWnHpWg8P3RP+hniqci8e3Q3YFODivR7QgNUK/DeRqDc0eEad5
+rBSgka8LuPGlhiOes67PojwIFV7Xw5Ndu1ePT7IRP7FNbrWO+tLQR41RvQlk45ne
+Qison6n8TF+vbaN6z0mCa+v21KsoBYxQHM7IJ6pgMxg24aNW0WTk7PXdJp1LWRiJ
+uixlXjOcKWQXaP+HxiQuXGj7isvv0O6xH2cl3GfgQ5rx8mG/APvLIz+dc/QBGQAr
+v6IVlDtd3AiYS7YeB7/5OvY+0emJ7U15ZJLNnCzlrqNDjxCN+cbXAeTdlKRJp21F
+rpjiZdfhAoGBAPw7EbWMq9ooluQ0bs+v6tvNCvXBfd/VAvG3w1/z3MvhVVYLx1Ag
+zleZom3YUXRv24WW3qHXFEGgyz2Sd3mJ4AuR9TDhvij6rHO6E0C8shB0oBlLoNo0
+4Ve28VQfaI77AKd7BYIoCWsCA5oTV+34AYlApMYkkaplRwSs9X5wIZ8ZAoGBAMGE
+7I1ASqMnQqdjzpBpGom+XpSPXGdBiH9mNPUajb0sPFvnnhpTSa3/k9m036Q9vQNH
+PEOeyjFbF7c/QKsPZLUbFl4uXdEmN4BUab5qQMSQVB9SlQOUB5G/qk/M90TgSbBm
+hFrpJrlf0Vsgnm1EMMOhoGdXbkB147AFnOcIekSFAoGAa31c0arOPd1YWI5Dvvxw
+MRWTmyHHW9EyPQKcH1MUgEpaDJ5eZTZl2Q0fHIK4S8+zlJ2z6PJ4rnMwyd+WTNRG
+B4g/HoLFgD87qOHefJMtqzeYVs9VEEjC05eiBsCP1YcAQ194/HvFb7XfBRVDPqWX
+Of+zeMFy1lPszQBMaoKswVkCgYAElrRNPSMH71xjP7icMAHTFlKDz0pvoFwuOSw0
+S6bkv3HG9B0JnsP2fkLxPJq4+EXNGBlTuSYuOWy8iaFs7PaEXNoQ7aSH2xIh1t6T
+B0312z5DZ9/kr9PmHtdZAREz7uWQaz3kMfcbGiyKrqFTEfTeDq0RBj+1A5aci+WG
+jOrpSQKBgQCvf/R/le3m8EsMe5AmNMl1mvpZ5wWn0yVS0vnjJRbS4TCUGK1lSf74
+tIZ+PEMp9CRaS2eTGtsWwQvuORlWlgYuYvJfxwvvnbLjln66SuE7pZHG2UILw4vZ
+5xkxTmL93VXFWRaH418mifGDiLIYr14+yzbW366r9L72BuN1dZJNzg==
+-----END RSA PRIVATE KEY-----