Skip to content

Method signature inconsistency - setNotFoundHandler #84

Closed
literakl opened this issue Mar 21, 2020 · 3 comments
Closed

Method signature inconsistency - setNotFoundHandler #84

literakl opened this issue Mar 21, 2020 · 3 comments

Comments

@literakl
Copy link

Every method in API has this sequence: req, res but notFound handler has an opposite

  errorHandlerCallback: (
    err: Error,
    req: HttpRequest,
    res: HttpResponse
  ) => HttpResponse

  validationErrorHandlerCallback: (
    errors: validationErrors,
    req: HttpRequest,
    res: HttpResponse
  ) => any

type HttpRoute = (
req: HttpRequest,
res: HttpResponse,
next?: Function,
config?: object,
previousMiddlewareResult?: any
) => any;

  notFoundHandlerCallback: (
    res: HttpResponseBasic,
    req: HttpRequestBasic
  ) => HttpRequestBasic

There is not much users yet, so you can break a backward compatibility and unify it.

@dalisoft
Copy link
Member

Yes, currently notFound handler does not support .header, .status, etc, so this method definition is correct currently, users can send at high-performance as i remember something was caused this leave this method as is.
On PRO version (PRO is re-written simple version) the bug is fixed by using Route instance, in free version there no Route instance.

So, let's finalize:

  1. Declaration was defined correctly
  2. Not found handler works
  3. Who needs modifications, uses uWS Http(Request, Response) instances to get what they need

@literakl
Copy link
Author

You did not understand me. I was complaining about an parameter order. All methods have (req, res), but this particular method has opposite order (res, req). Good API is consistent.

@dalisoft
Copy link
Member

Ah, now understand. I will fix it, but later, after some tests

dalisoft added a commit that referenced this issue Mar 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants