diff --git a/examples/express-sample/package-lock.json b/examples/express-sample/package-lock.json index 89cd721..0c063b5 100644 --- a/examples/express-sample/package-lock.json +++ b/examples/express-sample/package-lock.json @@ -22,7 +22,7 @@ }, "../..": { "name": "raygun", - "version": "0.14.0", + "version": "0.15.0-0", "dependencies": { "@types/express": "^4.17.21", "debug": "^4.3.4", @@ -31,8 +31,8 @@ "uuid": "^9.0.1" }, "devDependencies": { - "@eslint/js": "^9.1.1", - "@types/node": "^20.12.7", + "@eslint/js": "^9.2.0", + "@types/node": "^20.12.8", "@types/stack-trace": "0.0.33", "@types/uuid": "^9.0.8", "eslint": "^8.57.0", @@ -45,7 +45,7 @@ "tap": "^18.7.2", "ts-node": "^10.9.2", "typescript": "^5.4.5", - "typescript-eslint": "^7.7.1", + "typescript-eslint": "^7.8.0", "verror": "^1.10.1" }, "engines": { diff --git a/examples/using-domains/package-lock.json b/examples/using-domains/package-lock.json index 67ab0cd..de945f5 100644 --- a/examples/using-domains/package-lock.json +++ b/examples/using-domains/package-lock.json @@ -15,7 +15,7 @@ }, "../..": { "name": "raygun", - "version": "0.13.2", + "version": "0.15.0-0", "dependencies": { "@types/express": "^4.17.21", "debug": "^4.3.4", @@ -24,8 +24,8 @@ "uuid": "^9.0.1" }, "devDependencies": { - "@eslint/js": "^9.1.1", - "@types/node": "^20.12.7", + "@eslint/js": "^9.2.0", + "@types/node": "^20.12.8", "@types/stack-trace": "0.0.33", "@types/uuid": "^9.0.8", "eslint": "^8.57.0", @@ -38,7 +38,7 @@ "tap": "^18.7.2", "ts-node": "^10.9.2", "typescript": "^5.4.5", - "typescript-eslint": "^7.7.1", + "typescript-eslint": "^7.8.0", "verror": "^1.10.1" }, "engines": { diff --git a/lib/raygun.batch.ts b/lib/raygun.batch.ts index f08d72b..0fabff2 100644 --- a/lib/raygun.batch.ts +++ b/lib/raygun.batch.ts @@ -45,18 +45,28 @@ export class RaygunBatchTransport { /** * Enqueues send request to batch processor. - * Callback in SendOptions is called when the message is eventually processed. - * @param options + * @param options send options without callback + * @return Promise with response or error if rejected */ - send(options: SendOptions) { - this.onIncomingMessage({ - serializedMessage: options.message, - callback: options.callback, + send(options: SendOptions): Promise { + return new Promise((resolve, reject) => { + this.onIncomingMessage({ + serializedMessage: options.message, + // TODO: Switch to using Promises internally + // See issue: https://github.com/MindscapeHQ/raygun4node/issues/199 + callback: (error, message) => { + if (error) { + reject(error); + } else if (message) { + resolve(message); + } + }, + }); + + if (!this.timerId) { + this.timerId = setTimeout(() => this.processBatch(), 1000); + } }); - - if (!this.timerId) { - this.timerId = setTimeout(() => this.processBatch(), 1000); - } } stopProcessing() { @@ -76,11 +86,9 @@ export class RaygunBatchTransport { const messageSize = Math.ceil(messageLength / 1024); const startOfMessage = serializedMessage.slice(0, 1000); - console.warn( - `[raygun4node] Error is too large to send to Raygun (${messageSize}kb)\nStart of error: ${startOfMessage}`, - ); - - return; + const errorMessage = `Error is too large to send to Raygun (${messageSize}kb)\nStart of error: ${startOfMessage}`; + console.error(`[Raygun4Node] ${errorMessage}`); + throw Error(errorMessage); } const messageIsTooLargeToAddToBatch = @@ -151,6 +159,7 @@ export class RaygunBatchTransport { } // TODO: Callbacks are processed in batch, see how can this be implemented with Promises + // See issue: https://github.com/MindscapeHQ/raygun4node/issues/199 for (const callback of callbacks) { if (callback) { callVariadicCallback(callback, err, response); @@ -166,8 +175,15 @@ export class RaygunBatchTransport { sendBatch({ message: payload, - callback: runAllCallbacks, http: this.httpOptions, - }); + }) + .then((response) => { + // Call to original callbacks for success + runAllCallbacks(null, response); + }) + .catch((error) => { + // Call to original callbacks for error + runAllCallbacks(error, null); + }); } } diff --git a/lib/raygun.offline.ts b/lib/raygun.offline.ts index 7680a58..5f39bb3 100644 --- a/lib/raygun.offline.ts +++ b/lib/raygun.offline.ts @@ -32,8 +32,9 @@ export class OfflineStorage implements IOfflineStorage { path.join(this.cachePath, item), "utf8", (err, cacheContents) => { - // TODO: MessageTransport ignores any errors from the send callback, could this be improved? + // Attempt to send stored messages after going online this.transport.send(cacheContents); + // Ignore result, delete stored content nevertheless fs.unlink(path.join(this.cachePath, item), () => {}); }, ); @@ -63,13 +64,12 @@ export class OfflineStorage implements IOfflineStorage { fs.readdir(this.cachePath, (err, files) => { if (err) { - console.log(`[Raygun4Node] Error reading cache folder`); - console.log(err); + console.error(`[Raygun4Node] Error reading cache folder`, err); return callback(err); } if (files.length > this.cacheLimit) { - console.log(`[Raygun4Node] Error cache reached limit`); + console.error(`[Raygun4Node] Error cache reached limit`); return callback(null); } @@ -81,9 +81,7 @@ export class OfflineStorage implements IOfflineStorage { return callback(null); } - console.log(`[Raygun4Node] Error writing to cache folder`); - console.log(err); - + console.error(`[Raygun4Node] Error writing to cache folder`, err); return callback(err); }); }); @@ -98,8 +96,7 @@ export class OfflineStorage implements IOfflineStorage { send(callback: (error: Error | null, items?: string[]) => void) { this.retrieve((err, items) => { if (err) { - console.log(`[Raygun4Node] Error reading cache folder`); - console.log(err); + console.error(`[Raygun4Node] Error reading cache folder`, err); return callback(err); } diff --git a/lib/raygun.sync.transport.ts b/lib/raygun.sync.transport.ts index 9ce871c..a5e8571 100644 --- a/lib/raygun.sync.transport.ts +++ b/lib/raygun.sync.transport.ts @@ -9,11 +9,11 @@ "use strict"; import { spawnSync } from "child_process"; -import { SendOptionsWithoutCB } from "./types"; +import { SendOptions } from "./types"; const requestProcessSource = require.resolve("./raygun.sync.worker"); -function syncRequest(httpOptions: SendOptionsWithoutCB) { +function syncRequest(httpOptions: SendOptions) { const requestProcess = spawnSync("node", [requestProcessSource], { input: JSON.stringify(httpOptions), }); @@ -27,7 +27,7 @@ function syncRequest(httpOptions: SendOptionsWithoutCB) { * Only used to report uncaught exceptions. * @param options */ -export function send(options: SendOptionsWithoutCB) { +export function send(options: SendOptions) { try { syncRequest(options); } catch (e) { diff --git a/lib/raygun.sync.worker.ts b/lib/raygun.sync.worker.ts index 8d42b93..fde4954 100644 --- a/lib/raygun.sync.worker.ts +++ b/lib/raygun.sync.worker.ts @@ -1,22 +1,19 @@ import fs from "fs"; import * as transport from "./raygun.transport"; -import { SendOptions, SendOptionsWithoutCB } from "./types"; -import { IncomingMessage } from "http"; +import { SendOptions } from "./types"; // Read stdin synchronously const data = fs.readFileSync(0, "utf-8"); -const options: SendOptionsWithoutCB = JSON.parse(data); -const sendOptions: SendOptions = { ...options, callback }; +const options: SendOptions = JSON.parse(data); -transport.send(sendOptions); - -function callback(error: Error | null, result: IncomingMessage | null) { - if (error) { - console.log(`[Raygun4Node] Error sending with sync transport`, error); - } else { +transport + .send(options) + .then((response) => { console.log( `[Raygun4Node] Successfully reported uncaught exception to Raygun`, ); - } -} + }) + .catch((error) => { + console.error(`[Raygun4Node] Error sending with sync transport`, error); + }); diff --git a/lib/raygun.transport.ts b/lib/raygun.transport.ts index edc2aee..3cd82f9 100644 --- a/lib/raygun.transport.ts +++ b/lib/raygun.transport.ts @@ -12,27 +12,27 @@ import http from "http"; import https from "https"; import { IncomingMessage } from "http"; -import { - isCallbackWithError, - callVariadicCallback, - SendOptions, -} from "./types"; +import { SendOptions } from "./types"; const API_HOST = "api.raygun.io"; const DEFAULT_ENDPOINT = "/entries"; const BATCH_ENDPOINT = "/entries/bulk"; -export function sendBatch(options: SendOptions) { +export function sendBatch(options: SendOptions): Promise { return send(options, BATCH_ENDPOINT); } -// TODO: Convert this method callbacks to Promise. /** * Transport implementation that sends error to Raygun. * Errors are reported back via callback. - * @param options + * @param options without callback + * @param path service endpoint + * @return Promise with IncomingMessage or rejected with Error */ -export function send(options: SendOptions, path = DEFAULT_ENDPOINT) { +export function send( + options: SendOptions, + path = DEFAULT_ENDPOINT, +): Promise { try { const data = Buffer.from(options.message); @@ -49,32 +49,33 @@ export function send(options: SendOptions, path = DEFAULT_ENDPOINT) { }, }; - const cb = function (response: IncomingMessage) { - if (options.callback) { - return callVariadicCallback(options.callback, null, response); - } - }; + // Wrap HTTP request in Promise + return new Promise((resolve, reject) => { + const httpLib = options.http.useSSL ? https : http; + const request = httpLib.request( + httpOptions, + (response: IncomingMessage) => { + // request completed successfully + resolve(response); + }, + ); - const httpLib = options.http.useSSL ? https : http; - const request = httpLib.request(httpOptions, cb); + request.on("error", function (e) { + console.error( + `[Raygun4Node] error ${e.message} occurred while attempting to send error with message: ${options.message}`, + ); - request.on("error", function (e) { - console.log( - `[Raygun4Node] Error ${e.message} occurred while attempting to send error with message: ${options.message}`, - ); + // request failed + reject(e); + }); - // If the callback has two parameters, it should expect an `error` value. - if (options.callback && isCallbackWithError(options.callback)) { - options.callback(e, null); - } + request.write(data); + request.end(); }); - - request.write(data); - request.end(); } catch (e) { - // TODO: Non-HTTP errors are being ignored, should be better pass them up? - console.log( - `[Raygun4Node] Error ${e} occurred while attempting to send error with message: ${options.message}`, + console.error( + `[Raygun4Node] error ${e} occurred while attempting to send error with message: ${options.message}`, ); + return Promise.reject(e); } } diff --git a/lib/raygun.ts b/lib/raygun.ts index 4e96926..8de0dcd 100644 --- a/lib/raygun.ts +++ b/lib/raygun.ts @@ -200,38 +200,9 @@ class Raygun { request?: RequestParams, tags?: Tag[], ): Promise { - // Convert internal sendWithCallback implementation to a Promise. - return new Promise((resolve, reject) => { - this.sendWithCallback( - exception, - customData, - function (err, message) { - if (err != null) { - reject(err); - } else { - resolve(message); - } - }, - request, - tags, - ); - }); - } - - /** - * @deprecated sendWithCallback is a deprecated method. Instead, use send, which supports async/await calls. - */ - sendWithCallback( - exception: Error | string, - customData?: CustomData, - callback?: Callback, - request?: RequestParams, - tags?: Tag[], - ): Message { const sendOptionsResult = this.buildSendOptions( exception, customData, - callback, request, tags, ); @@ -240,26 +211,75 @@ class Raygun { if (!sendOptionsResult.valid) { console.error( - `[Raygun4Node] Encountered an error sending an error to Raygun. No API key is configured, please ensure .init() is called with api key. See docs for more info.`, + `[Raygun4Node] Encountered an error sending an error to Raygun. No API key is configured, please ensure .init is called with api key. See docs for more info.`, ); - return sendOptionsResult.message; + return Promise.reject(sendOptionsResult.message); } const sendOptions = sendOptionsResult.options; - if (this._isOffline) { - // make the save callback type compatible with Callback - const saveCallback = callback - ? (err: Error | null) => callVariadicCallback(callback, err, null) - : emptyCallback; - this.offlineStorage().save(JSON.stringify(message), saveCallback); + // Server is offline, store in Offline Storage + return new Promise((resolve, reject) => { + this.offlineStorage().save(JSON.stringify(message), (error) => { + if (error) { + console.error( + "[Raygun4Node] Error storing message while offline", + error, + ); + reject(error); + } else { + debug("[raygun.ts] Stored message while offline"); + // Resolved value is null because message is stored + resolve(null); + } + }); + }); } else { + // wrap Promise and add duration debug info + const stopTimer = startTimer(); // Use current transport to send request. // Transport can be batch or default. - this.transport().send(sendOptions); + return this.transport() + .send(sendOptions) + .then((response) => { + const durationInMs = stopTimer(); + debug( + `[raygun.ts] Successfully sent message (duration=${durationInMs}ms)`, + ); + return response; + }) + .catch((error) => { + const durationInMs = stopTimer(); + debug( + `[raygun.ts] Error sending message (duration=${durationInMs}ms): ${error}`, + ); + return error; + }); } + } - return message; + /** + * @deprecated sendWithCallback is a deprecated method. Instead, use send, which supports async/await calls. + */ + sendWithCallback( + exception: Error | string, + customData?: CustomData, + callback?: Callback, + request?: RequestParams, + tags?: Tag[], + ) { + // call async send but redirect response to provided legacy callback + this.send(exception, customData, request, tags) + .then((response) => { + if (callback) { + callVariadicCallback(callback, null, response); + } + }) + .catch((error) => { + if (callback) { + callVariadicCallback(callback, error, null); + } + }); } private reportUncaughtExceptions() { @@ -321,7 +341,7 @@ class Raygun { this.send(err, customData || {}, requestParams, [ "UnhandledException", ]).catch((err) => { - console.log(`[Raygun] Failed to send Express error: ${err}`); + console.error(`[Raygun] Failed to send Express error`, err); }); next(err); } @@ -336,7 +356,6 @@ class Raygun { private buildSendOptions( exception: Error | string, customData?: CustomData, - callback?: Callback, request?: RequestParams, tags?: Tag[], ): SendOptionsResult { @@ -394,34 +413,11 @@ class Raygun { return { valid: false, message }; } - function wrappedCallback( - error: Error | null, - response: IncomingMessage | null, - ) { - const durationInMs = stopTimer(); - if (error) { - debug( - `[raygun.ts] Error sending message (duration=${durationInMs}ms): ${error}`, - ); - } else { - debug( - `[raygun.ts] Successfully sent message (duration=${durationInMs}ms)`, - ); - } - if (!callback) { - return; - } - return callVariadicCallback(callback, error, response); - } - - const stopTimer = startTimer(); - return { valid: true, message, options: { message: JSON.stringify(message), - callback: wrappedCallback, http: { host: this._host, port: this._port, @@ -442,13 +438,23 @@ class Raygun { }; return { - // TODO: MessageTransport ignores any errors from the send callback, could this be improved? send(message: string) { - transport.send({ - message, - callback: () => {}, - http: httpOptions, - }); + transport + .send({ + message, + http: httpOptions, + }) + .then((response) => { + debug( + `[raygun.ts] Sent message from offline transport: ${response}`, + ); + }) + .catch((error) => { + console.error( + `[Raygun4Node] Failed to send message from offline transport`, + error, + ); + }); }, }; } diff --git a/lib/types.ts b/lib/types.ts index 3c9ebda..7fc506a 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -68,13 +68,9 @@ export type Tag = string; export type SendOptions = { message: string; - // TODO: Remove Callback in SendOptions and use Promises internally - callback: Callback; http: HTTPOptions; }; -export type SendOptionsWithoutCB = Omit; - export type HTTPOptions = { useSSL: boolean; host: string | undefined; @@ -132,7 +128,7 @@ export type OfflineStorageOptions = { }; export type Transport = { - send(options: SendOptions): void; + send(options: SendOptions): Promise; }; export type MessageTransport = { diff --git a/test/raygun_async_send_test.js b/test/raygun_async_send_test.js index 3d270c2..a94c6e4 100644 --- a/test/raygun_async_send_test.js +++ b/test/raygun_async_send_test.js @@ -158,3 +158,22 @@ test("check that tags get merged", {}, function (t) { t.fail(err); }); }); + +test("send with expressHandler custom data", function (t) { + t.plan(1); + let client = new Raygun.Client().init({ + apiKey: API_KEY, + }); + + client.expressCustomData = function () { + return { test: "data" }; + }; + client._send = client.send; + client.send = (err, data, params, tags) => { + client.send = client._send; + t.equal(data.test, "data"); + t.end(); + return Promise.resolve(null); + }; + client.expressHandler(new Error(), {}, {}, function () {}); +}); diff --git a/test/raygun_send_test.js b/test/raygun_send_test.js index 7fa9a6b..bd0035c 100644 --- a/test/raygun_send_test.js +++ b/test/raygun_send_test.js @@ -126,24 +126,6 @@ test("send with OnBeforeSend", {}, function (t) { }); }); -test("send with expressHandler custom data", function (t) { - t.plan(1); - var client = new Raygun.Client().init({ - apiKey: API_KEY, - }); - - client.expressCustomData = function () { - return { test: "data" }; - }; - client._send = client.sendWithCallback; - client.sendWithCallback = function (err, data) { - client.sendWithCallback = client._send; - t.equal(data.test, "data"); - t.end(); - }; - client.expressHandler(new Error(), {}, {}, function () {}); -}); - test("check that tags get passed through", {}, function (t) { var tag = ["Test"]; var client = new Raygun.Client().init({ apiKey: "TEST" });