improved node transport error handling

things were broken after pagination refactoring. Now they're fixed and improved.

packages/sdk-node/src/nodeTransport.spec.ts

@@ -57,9 +57,7 @@ describe('NodeTransport', () => {
       expect(response.ok).toEqual(false)
       expect(response.statusCode).toEqual(404)
       expect(response.body).toBeDefined()
-      expect(response.body.indexOf(errorMessage)).toEqual(0)
-      expect(response.body.length).toBeGreaterThan(0)
-      expect(response.statusMessage.indexOf('"type":"Buffer":')).toEqual(-1)
+      expect(response.statusMessage.indexOf('"type":"Buffer"')).toEqual(-1)
       expect(response.statusMessage.indexOf(errorMessage)).toEqual(0)
     })
   })

packages/sdk-node/src/nodeTransport.ts

@@ -52,6 +52,18 @@ import type {
   ICryptoHash,
 } from '@looker/sdk-rtl'
 
+const utf8 = 'utf8'
+
+const asString = (value: any): string => {
+  if (value instanceof Buffer) {
+    return Buffer.from(value).toString(utf8)
+  }
+  if (value instanceof Object) {
+    return JSON.stringify(value)
+  }
+  return value.toString()
+}
+
 export class NodeCryptoHash implements ICryptoHash {
   secureRandom(byteCount: number): string {
     return nodeCrypto.randomBytes(byteCount).toString('hex')
@@ -64,7 +76,9 @@ export class NodeCryptoHash implements ICryptoHash {
   }
 }
 
-export type RequestOptions = rq.RequiredUriUrl & rp.RequestPromiseOptions
+export type RequestOptions = rq.RequiredUriUrl &
+  rp.RequestPromiseOptions &
+  rq.OptionsWithUrl
 
 export class NodeTransport extends BaseTransport {
   constructor(protected readonly options: ITransportSettings) {
@@ -79,7 +93,7 @@ export class NodeTransport extends BaseTransport {
     authenticator?: Authenticator,
     options?: Partial<ITransportSettings>
   ): Promise<IRawResponse> {
-    const init = await this.initRequest(
+    const init: RequestOptions = await this.initRequest(
       method,
       path,
       queryParams,
@@ -93,7 +107,7 @@ export class NodeTransport extends BaseTransport {
       const res = await req
       const resTyped = res as rq.Response
       rawResponse = {
-        url: resTyped.url || '',
+        url: resTyped.url || init.url.toString() || '',
         body: await resTyped.body,
         contentType: String(resTyped.headers['content-type']),
         ok: true,
@@ -102,32 +116,31 @@ export class NodeTransport extends BaseTransport {
         headers: res.headers,
       }
     } catch (e) {
-      const statusMessage = `${method} ${path}`
+      let statusMessage = `${method} ${path}`
       let statusCode = 404
       let contentType = 'text'
-      let body: string
+      let body
       if (e instanceof StatusCodeError) {
         statusCode = e.statusCode
-        const text = e.message
-        body = e.message
-        // Need to re-parse the error message
-        const matches = /^\d+\s*-\s*({.*})/gim.exec(text)
-        if (matches && matches.length > 1) {
-          const json = matches[1]
-          const obj = JSON.parse(json)
-          if (obj.data) {
-            body = Buffer.from(obj.data).toString('utf8')
-          }
+        if (e.error instanceof Buffer) {
+          body = asString(e.error)
+          statusMessage += `: ${statusCode}`
+        } else if (e.error instanceof Object) {
+          // Capture error object as body
+          body = e.error
+          statusMessage += `: ${e.message}`
+          // Clarify the error message
+          body.message = statusMessage
+          contentType = 'application/json'
         }
-        body = `${statusMessage} ${body}`
       } else if (e.error instanceof Buffer) {
-        body = Buffer.from(e.error).toString('utf8')
+        body = asString(e.error)
       } else {
         body = JSON.stringify(e)
         contentType = 'application/json'
       }
       rawResponse = {
-        url: this.makeUrl(path, { ...this.options, ...options }, queryParams),
+        url: init.url.toString(),
         body,
         contentType,
         ok: false,
@@ -141,35 +154,39 @@ export class NodeTransport extends BaseTransport {
 
   async parseResponse<TSuccess, TError>(res: IRawResponse) {
     const mode = responseMode(res.contentType)
-    const utf8 = 'utf8'
-    let result = await res.body
+    let response: SDKResponse<TSuccess, TError>
     let error
+    if (!res.ok) {
+      // Raw request had an error. Make sure it's a string before parsing the result
+      error = res.body
+      if (typeof error === 'string') error = JSON.parse(error)
+      response = { ok: false, error }
+      return response
+    }
+    let result = await res.body
     if (mode === ResponseMode.string) {
       if (res.contentType.match(/^application\/.*\bjson\b/g)) {
         try {
           if (result instanceof Buffer) {
-            result = (result as Buffer).toString(utf8)
+            result = Buffer.from(result).toString(utf8)
           }
           if (!(result instanceof Object)) {
             result = JSON.parse(result.toString())
           }
         } catch (err) {
           error = err
         }
-      } else if (result instanceof Buffer) {
-        result = (result as Buffer).toString(utf8)
-      }
-      if (!error) {
-        result = result.toString()
+      } else if (!error) {
+        // Convert to string otherwise
+        result = asString(result)
       }
     } else {
       try {
-        result = (result as Buffer).toString('binary')
+        result = Buffer.from(result).toString('binary')
       } catch (err) {
         error = err
       }
     }
-    let response: SDKResponse<TSuccess, TError>
     if (!error) {
       response = { ok: true, value: result }
     } else {

packages/sdk-rtl/src/transport.ts

@@ -431,6 +431,7 @@ export function addQueryParams(path: string, obj?: Values) {
  * @returns a new `Error` object with the failure message
  */
 export function sdkError(response: any) {
+  const utf8 = 'utf-8'
   if (typeof response === 'string') {
     return new Error(response)
   }
@@ -444,11 +445,11 @@ export function sdkError(response: any) {
       return new Error(error.statusMessage)
     }
     if ('error' in error && error.error instanceof Buffer) {
-      const result = Buffer.from(error.error).toString('utf-8')
+      const result = Buffer.from(error.error).toString(utf8)
       return new Error(result)
     }
     if (error instanceof Buffer) {
-      const result = Buffer.from(error).toString('utf-8')
+      const result = Buffer.from(error).toString(utf8)
       return new Error(result)
     }
     if ('message' in error) {