Merge pull request #151 from supercharge/error-handler-switch-params

Error handler: switch error and ctx params

Author: marcuspoehls

CHANGELOG.md

@@ -47,6 +47,20 @@
 - `@supercharge/http`
     - the `RequestHeaderBag` extends the `InputBag` which changes the behavior of the `has(key)` method: it returns `false` if the stored value is `undefined` and returns `true` otherwise
 
+    This pull request switches the parameters of the error report, handle, render method:
+- `@supercharge/core`
+    - switch the parameters of the error’s and error handler’s `report`, `handle`, `render` methods:
+    ```ts
+    // before
+    handle(ctx: HttpContext, error: Error)
+    report(ctx: HttpContext, error: Error)
+    render(ctx: HttpContext, error: Error)
+
+    // now
+    handle(error: Error, ctx: HttpContext)
+    report(error: Error, ctx: HttpContext)
+    render(error: Error, ctx: HttpContext)
+    ```
 
 
 ## [3.20.4](https://github.com/supercharge/framework/compare/v3.20.3...v3.20.4) - 2023-10-15

packages/contracts/src/core/error-handler.ts

@@ -16,15 +16,15 @@ export interface ErrorHandler {
   /**
    * Handle the given error.
    */
-  handle(ctx: HttpContext, error: Error): Promise<void>
+  handle(error: Error, ctx: HttpContext): Promise<void>
 
   /**
    * Report an error.
    */
-  report(context: HttpContext, error: Error): void | Promise<void>
+  report(error: Error, ctx: HttpContext): void | Promise<void>
 
   /**
    * Render an error into an HTTP response.
    */
-  render(context: HttpContext, error: Error): Promise<any>
+  render(error: Error, ctx: HttpContext): Promise<any>
 }

packages/core/src/errors/handler.ts

@@ -19,7 +19,7 @@ export class ErrorHandler implements ErrorHandlerContract {
   /**
    * Stores the list of report callbacks.
    */
-  protected readonly reportCallbacks: Array<(ctx: HttpContext, error: HttpError) => void | Promise<void>>
+  protected readonly reportCallbacks: Array<(error: HttpError, ctx: HttpContext) => void | Promise<void>>
 
   /**
    * Create a new error handler instance.
@@ -85,7 +85,7 @@ export class ErrorHandler implements ErrorHandlerContract {
   /**
    * Register a reportable callback.
    */
-  reportable (reportUsing: (ctx: HttpContext, error: any) => void | Promise<void>): ErrorHandler {
+  reportable (reportUsing: (error: any, ctx: HttpContext) => void | Promise<void>): ErrorHandler {
     return tap(this, () => {
       this.reportCallbacks.push(reportUsing)
     })
@@ -112,25 +112,25 @@ export class ErrorHandler implements ErrorHandlerContract {
   /**
    * Handle the given error.
    */
-  async handle (ctx: HttpContext, error: any): Promise<void> {
-    await this.report(ctx, error)
-    await this.render(ctx, error)
+  async handle (error: any, ctx: HttpContext): Promise<void> {
+    await this.report(error, ctx)
+    await this.render(error, ctx)
   }
 
   /**
    * Report an error.
    */
-  async report (ctx: HttpContext, error: any): Promise<void> {
+  async report (error: any, ctx: HttpContext): Promise<void> {
     if (this.shouldNotReport(error)) {
       return
     }
 
-    if (await this.errorReported(ctx, error)) {
+    if (await this.errorReported(error, ctx)) {
       return
     }
 
     const handled = await Collect(this.reportCallbacks).any(async reportCallback => {
-      return await reportCallback(ctx, error)
+      return await reportCallback(error, ctx)
     })
 
     if (handled) {
@@ -147,54 +147,54 @@ export class ErrorHandler implements ErrorHandlerContract {
   }
 
   /**
-   * Determine whether the given `error` is implementing a `handle` method and
-   * that `handle` method returns a truthy value, like a valid HTTP response.
+   * Determine whether the given `error` is implementing a `report` method and
+   * that `report` method returns a truthy value, like a valid HTTP response.
    */
-  async errorReported (ctx: HttpContext, error: any): Promise<unknown> {
+  async errorReported (error: any, ctx: HttpContext): Promise<unknown> {
     if (typeof error.report !== 'function') {
       return false
     }
 
-    return await error.report(error, ctx)
+    return !!(await error.report(error, ctx))
   }
 
   /**
    * Render the error into an HTTP response.
    */
-  async render (ctx: HttpContext, error: any): Promise<any> {
-    if (await this.errorRendered(ctx, error)) {
+  async render (error: any, ctx: HttpContext): Promise<any> {
+    if (await this.errorRendered(error, ctx)) {
       return
     }
 
     const httpError = HttpError.wrap(error)
 
     if (ctx.request.wantsJson()) {
-      return this.renderJsonResponse(ctx, httpError)
+      return this.renderJsonResponse(httpError, ctx)
     }
 
     if (this.app.env().isProduction()) {
-      return this.renderJsonResponse(ctx, httpError)
+      return this.renderJsonResponse(httpError, ctx)
     }
 
-    return await this.renderViewResponse(ctx, httpError)
+    return await this.renderViewResponse(httpError, ctx)
   }
 
   /**
    * Determine whether the given `error` is implementing a `render` method and
    * that `render` method returns a truthy value, like a valid HTTP response.
    */
-  async errorRendered (ctx: HttpContext, error: any): Promise<unknown> {
+  async errorRendered (error: any, ctx: HttpContext): Promise<unknown> {
     if (typeof error.render !== 'function') {
       return false
     }
 
-    return await error.render(ctx, error)
+    return await error.render(error, ctx)
   }
 
   /**
    * Creates a JSON response depending on the app’s environment.
    */
-  protected renderJsonResponse (ctx: HttpContext, error: HttpError): void {
+  protected renderJsonResponse (error: HttpError, ctx: HttpContext): void {
     const { message, stack, status: statusCode } = error
 
     this.app.env().isProduction()
@@ -205,7 +205,7 @@ export class ErrorHandler implements ErrorHandlerContract {
   /**
    * Creates an HTML response depending on the app’s environment.
    */
-  protected async renderViewResponse (ctx: HttpContext, error: HttpError): Promise<void> {
+  protected async renderViewResponse (error: HttpError, ctx: HttpContext): Promise<void> {
     if (await this.isMissingTemplateFor(error)) {
       return await this.renderYouchResponse(ctx, error)
     }

packages/core/test/error-handler.js

@@ -183,7 +183,7 @@ test('calls report callbacks', async () => {
 
   class CustomErrorHandler extends ErrorHandler {
     register () {
-      this.reportable((_, error) => {
+      this.reportable(error => {
         reportedError = error
       })
     }
@@ -215,7 +215,7 @@ test('report callbacks can stop the reportable chain', async () => {
         .reportable(() => {
           return true
         })
-        .reportable((_ctx, error) => {
+        .reportable(error => {
           reportedError = error
         })
     }
@@ -448,7 +448,7 @@ class ReportedError extends ReportingError {
 }
 
 class RenderError extends Error {
-  render (ctx, error) {
+  render (error, ctx) {
     return ctx.response.payload({
       message: error.message,
       foo: 'bar',

packages/http/src/middleware/handle-error.ts

@@ -10,16 +10,16 @@ export class HandleErrorMiddleware extends Middleware {
     try {
       await next()
     } catch (error: any) {
-      await this.handleError(ctx, error)
+      await this.handleError(error, ctx)
     }
   }
 
   /**
    * Process the given `error` and HTTP `ctx` using the error handler.
    */
-  private async handleError (ctx: HttpContext, error: Error): Promise<void> {
+  private async handleError (error: Error, ctx: HttpContext): Promise<void> {
     if (this.app.hasBinding('error.handler')) {
-      return await this.app.make<ErrorHandler>('error.handler').handle(ctx, error)
+      return await this.app.make<ErrorHandler>('error.handler').handle(error, ctx)
     }
 
     throw error

packages/http/test/helpers/error-handler.js

@@ -1,6 +1,6 @@
 
 export default class ErrorHandler {
-  handle (ctx, error) {
+  handle (error, ctx) {
     // console.log('Received error in testing error handler', { error })
 
     ctx.response.status(error.status || error.statusCode || 500)

packages/session/test/helpers/error-handler.js

@@ -2,9 +2,9 @@
 import { ErrorHandler as Handler } from '@supercharge/core'
 
 export default class ErrorHandler extends Handler {
-  handle (ctx, error) {
+  handle (error, ctx) {
     // console.error('Received error in testing error handler', { error })
 
-    return super.handle(ctx, error)
+    return super.handle(error, ctx)
   }
 }