Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Echo v5 #1302

Closed
vishr opened this issue Mar 7, 2019 · 38 comments
Closed

Echo v5 #1302

vishr opened this issue Mar 7, 2019 · 38 comments
Assignees
Labels

Comments

@vishr
Copy link
Member

vishr commented Mar 7, 2019

Let me know what you want to see in v5 ✌️

Here are my thoughts?

  • I don't like Context as an interface, we did it to expand on it using a custom context; however, casting it back in handler is tedious
  • Simplified middleware signature like in v1/v2? making it easier to write a middleware.
  • Bind path parameters
  • Merge feat: Add a new ErrorHandlerWithContext #1328 with JWTErrorHandler (How about adding next to the handler so you can rely on next auth handler)
@vishr vishr added the v5 label Mar 7, 2019
@vishr vishr self-assigned this Mar 7, 2019
@QuantumGhost
Copy link

I'd like to know if we can make #1214 in v5.

@zxysilent
Copy link

Consider using only one log module

@vishr
Copy link
Member Author

vishr commented Mar 9, 2019

Consider using only one log module

Which one?

@zxysilent
Copy link

Consider using only one log module

Which one?

image

@harshachandra
Copy link

StdLogger looks fine to me provided we have "bring your own logger" option is given.
And similarly can we bring own Router like we have option to set Validator?

@mei-rune
Copy link

mei-rune commented Mar 11, 2019

I like it Context as interface, but need improve now code.

add methods:

(e *Echo) SetNewContext(newCtx  func() Context) {
 e.pool.New = func() interface{} {
		return newCtx()
	}
}

func (e *Echo) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	c := e.pool.Get().(Context)
	c.Reset(r, w)
	......
	c.Free()
	e.pool.Put(c)
}

my client code:

type MyContext strcut {
	echo.Context
	
	MyField1 xxx
	MyField2 xxx
	MyField3 xxx
	.... xxx
}

func (ctx *MyContext) Free() {
   ctx.MyField1 = nil
   ctx.MyField2 = nil
   ctx.MyField3 = nil
   ....
}

func toEchoFunc(h func(ctx *MyContext) error) echo.HandlerFunc {
	return func(ctx echo.Context) error {
		return h(ctx.(*MyContext))
	}
}

echo.SetNew(func() echo.Context {
	return &MyContext{Context: echo.NewContext()}
})

echo.GET("/xxx", toEchoFunc(func(ctx *MyContext) error {
   xxxxxxx
}))

@mei-rune
Copy link

mei-rune commented Mar 11, 2019

add gin.IRoutes(iris.Party) interface.

  e := echo.New()
  initHandlers(e)     // now is error
  initHandlers(e.Group("/")) 
  ...

func initHandlers(g *echo.Group) {
  g.Get("/x", a)
  g.Get("/y", a)
}

@universonic
Copy link

How about #1261 ?

@mei-rune
Copy link

add NoRoute?
#900
#856

@awbraunstein
Copy link
Contributor

I think that echo.Context should implement context.Context. This way deadlines to backends could be correctly propagated as well as tracing data.

@harshachandra
Copy link

Can we have an examples section describing the existing APIs from https://godoc.org/gopkg.in/labstack/echo.v4 or going forward https://godoc.org/gopkg.in/labstack/echo.v5? They can be hosted from https://echo.labstack.com/

@alexaandru
Copy link
Contributor

  • I don't like Context as an interface, we did it to expand on it using a custom context; however, casting it back in handler is tedious.
    I do like it, the casting is a very small price to pay for the convenience of adding your own logic on top of echo.Context
  • Simplified middleware signature like in v1/v2? making it easier to write a middleware.
    We could move the middleware in a separate repo, each in their own subfolders/Go modules. That way they can evolve at different pace.

@mei-rune
Copy link

@aimeelaplant I don't llke that echo.Context should implement context.Context.
but I think context.Context as a Field in echo.Context is good idea.

@mei-rune
Copy link

Remove logger interface{} or simple it!
#1017

@RichyHBM
Copy link

A way to define routing from a config file? Can already print out routing config to JSON, so hopefully the opposite can be achieved! (Though hopefully something simpler than JSON)

A nice config schema could be the one Play Framework uses

@cousinbenson
Copy link
Contributor

cousinbenson commented Mar 25, 2019

binding path parameters?

@universonic
Copy link

universonic commented Mar 28, 2019

How about make the router more generic so that we can use it with many other protocols (such as gRPC, etc.), not just HTTP?

@nervgh
Copy link

nervgh commented Apr 10, 2019

Hi everyone!

I'm from Node.js's world and I've been looking at echo only a couple days. I've been using koa2 (the successor of express) for 2 years so I will be comparing them.

1. There is no any router in the box

Koa is just a middleware pipeline. You are free to use your favorite router or write your own from scratch. But there is a trick -- composition.

a) you can create three apps and mount one to another

const mount = require('koa-mount')
const Koa = require('koa')

const a = new Koa()
a.use(async function (ctx, next) {
  ctx.body = 'Hello'
})

const b = new Koa();
b.use(async function (ctx, next) {
  ctx.body = 'World'
})

// app (root)
const app = new Koa()

app.use(mount('/hello', a))
app.use(mount('/world', b))

app.listen(3000)
console.log('listening on port 3000')
$ GET /
Not Found

$ GET /hello
Hello

$ GET /world
World

https://github.com/koajs/mount#mounting-applications

b) you can mount middleware as is

const mount = require('koa-mount')
const Koa = require('koa')

async function hello(ctx, next) {
  ctx.body = 'Hello'
}

async function world(ctx, next) {
  ctx.body = 'World'
}

const app = new Koa()

app.use(mount('/hello', hello))
app.use(mount('/world', world))

app.listen(3000)
console.log('listening on port 3000')

https://github.com/koajs/mount#mounting-middleware

c) you can create a router and mount it as middleware

const Koa = require('koa')
const Router = require('koa-trie-router')

const app = new Koa()
const router = new Router()

router
  .use(function(ctx, next) {
    console.log('* requests')
    return next()
  })
  .get(function(ctx, next) {
    console.log('GET requests')
    return next()
  })
  .put('/foo', function (ctx) {
    ctx.body = 'PUT /foo requests'
  })
  .post('/bar', function (ctx) {
    ctx.body = 'POST /bar requests'
  })

app.use(router.middleware()) // or app.use(mount('/foo', router.middleware()))
app.listen(3000)

https://github.com/koajs/trie-router#usage

As opposite, Express has the built-in router. Why did the developers exclude it from the web-framework? Well, you can try to google the answer :)

2. Extremely simple middleware usage

Let's say I have three middleware. If I want to execute next middleware I just do return next() and go further:

const Koa = require('koa')

const app = new Koa()

app.use(async function (ctx, next) {
  console.log('middleware 1')
  return next() // go further
})
app.use(async function (ctx, next) {
  console.log('middleware 2')
  return // stop here
})
app.use(async function (ctx, next) {
  // this middleware will have never called
  console.log('middleware 3')
  ctx.body = 'Hello World'
})

app.listen(3000)

This code prints to stdout

middleware 1
middleware 2

Echo brings me several kinds of middleware. One when I want to pass through

app.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
	return func(c echo.Context) error {
		fmt.Println("test 1")
		return next(c)
	}
})

and another in other case

app.GET("/", func(c echo.Context) error {
	return c.String(http.StatusOK, "Hello, World!")
})

But as a developer I actually want something simple, something like this:

app.Use(func(ctx echo.Context, next echo.HandlerFunc) {
	fmt.Println("test 1")
	return next(ctx) 
})

3. Wrapping of Request and Response is not a bad idea

This approach allows us to decompose Request \ Response helpers from Context object:

app.use(async function (ctx) {
  ctx // is the Koa Context

  ctx.request // is a Koa Request
  ctx.response // is a Koa Response

  ctx.req // Node's request object
  ctx.res // Node's response object
})

https://koajs.com/#context

4. ctx.assert(exp, [status], [msg]) as the declarative approach for throwing errors

It is quite often when we check some business rules and throw errors. Pseudo code could look as follows:

if x != 2 then
   throw HTTP 400 error
end

We prefer do it as singe line via ctx.assert():

ctx.assert(x == 2, 400, /* [Custom message] */)

https://koajs.com/#ctx-assert-value-status-msg-properties-


P.S.: I don't know it will be helpful for you or not. Anyway thank you for your amazing work 👍

"There is no limit of perfection" :)

@vorlif
Copy link

vorlif commented Apr 29, 2019

First of all, thanks for echo. It is my absolute favorite.

So I personally think it would be nice if you could only use the router (without echos http.Server, net.Listener or autocert). So it would be easier to configure my own http.Server or to use other libraries like
quic-go , which also need an http.Handler.

I know it's possible to do current ones, but it would be nice if these parts were better separated.

@yankeguo
Copy link

yankeguo commented Jul 6, 2019

#1362 use http.FileSystem interface and support 3rd party implementation

@jgrossophoff
Copy link

No or few breaking changes instead of barely needed or easily self-implemented features.

@stale
Copy link

stale bot commented Oct 13, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 13, 2019
@vishr vishr removed the wontfix label Oct 13, 2019
@StevenACoffman
Copy link

Per #1017 I would like to reiterate this comment by @ghostsquad:
It would be great to standardize the logger interface and allow developers/users of echo to add any logger they wish.

If this is going to be done, I would strongly recommend not using the built-in go logger methods as a base for the interface. There are many examples out there that talk about the anti-pattern of using log methods as control flow, as is such with fatal and panic methods. Additionally, I've seen many threads about having too many or not enough logging levels.

go-logr is an opinionated logger interface (used by Kubernetes), which defines only info and error methods, and it's up to the implementer to make additional decisions (such as choosing not to log when extra fields such as "debug: true" are set) https://github.com/go-logr/logr/blob/master/logr.go

The basis of this interface is derived from the post made by Dave Cheney: https://dave.cheney.net/2015/11/05/lets-talk-about-logging

type InfoLogger interface {
	// Info logs a non-error message with the given key/value pairs as context.
	//
	// The msg argument should be used to add some constant description to
	// the log line.  The key/value pairs can then be used to add additional
	// variable information.  The key/value pairs should alternate string
	// keys and arbitrary values.
	Info(msg string, keysAndValues ...interface{})

	// Enabled tests whether this InfoLogger is enabled.  For example,
	// commandline flags might be used to set the logging verbosity and disable
	// some info logs.
	Enabled() bool
}

// Logger represents the ability to log messages, both errors and not.
type Logger interface {
	// All Loggers implement InfoLogger.  Calling InfoLogger methods directly on
	// a Logger value is equivalent to calling them on a V(0) InfoLogger.  For
	// example, logger.Info() produces the same result as logger.V(0).Info.
	InfoLogger

	// Error logs an error, with the given message and key/value pairs as context.
	// It functions similarly to calling Info with the "error" named value, but may
	// have unique behavior, and should be preferred for logging errors (see the
	// package documentations for more information).
	//
	// The msg field should be used to add context to any underlying error,
	// while the err field should be used to attach the actual error that
	// triggered this log line, if present.
	Error(err error, msg string, keysAndValues ...interface{})

	// V returns an InfoLogger value for a specific verbosity level.  A higher
	// verbosity level means a log message is less important.  It's illegal to
	// pass a log level less than zero.
	V(level int) InfoLogger

	// WithValues adds some key-value pairs of context to a logger.
	// See Info for documentation on how key/value pairs work.
	WithValues(keysAndValues ...interface{}) Logger

	// WithName adds a new element to the logger's name.
	// Successive calls with WithName continue to append
	// suffixes to the logger's name.  It's strongly reccomended
	// that name segments contain only letters, digits, and hyphens
	// (see the package documentation for more information).
	WithName(name string) Logger
}

or even simpler, is in go-kit https://github.com/go-kit/kit/blob/master/log/log.go

type Logger interface {
	Log(keyvals ...interface{}) error
}

With some additional methods available from the library such as With, WithPrefix.

@yjradeh
Copy link

yjradeh commented Nov 22, 2019

Dependency injection please

@koddr
Copy link

koddr commented Nov 24, 2019

Thanks for Echo 👍

IMHO, Echo v5 need to upgrade JSON Marshal/Unmarshal speed. For example, we can use json-iterator/go for replace standard encoding/json.

Here is benchmark:

And it's easy to replace, because json-iterator 100% compatibility with standard lib.

Replace

import "encoding/json"

json.Marshal(&data)

with

import "github.com/json-iterator/go"

var json = jsoniter.ConfigCompatibleWithStandardLibrary
json.Marshal(&data)

And replace

import "encoding/json"

json.Unmarshal(input, &data)

with

import "github.com/json-iterator/go"

var json = jsoniter.ConfigCompatibleWithStandardLibrary
json.Unmarshal(input, &data)

@vorlif
Copy link

vorlif commented Nov 24, 2019

And it's easy to replace, because json-iterator 100% compatibility with standard lib.

No, this is a known incorrectness.

@koddr
Copy link

koddr commented Nov 24, 2019

@vorlif

No, this is a known incorrectness.

But Gin use it via build flag $ go build -tags=jsoniter ., and (maybe) Echo dev team can try to do similar way. I just say what I want|need for more speed up Echo and turn my next production project to Echo from Gin :)

@arch-mage
Copy link

What about reducing logger interface? The current interface makes it cumbersome to implement a logger.

@WhileLoop
Copy link
Contributor

IPv6 support please. #826

@stale
Copy link

stale bot commented Feb 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Feb 22, 2020
@stale stale bot closed this as completed Feb 29, 2020
@vishr vishr removed the wontfix label Feb 29, 2020
@StarpTech
Copy link

Please reopen.

@zxysilent
Copy link

Do you have a development plan

@dom3k
Copy link

dom3k commented Oct 8, 2020

What is going on?

@duraki
Copy link

duraki commented Nov 27, 2020

GitHub issue bots are the worst thing that could happen to open-source community. Especially the @stale bot.

@lammel
Copy link
Contributor

lammel commented Nov 28, 2020

Yes, the stale bot is a little nasty sometimes, but also helpful in some situations.

But this discussion is about ideas and suggestions for v5. We labeled some issues and PRs already for the next major release and will start with actual development soon (after resolving open bind and router issues in v4). Some suggestions with no issues or PRs yet will be openend for a more detailed discussion and first PRs merged.

Anyone who wants it to move faster or wants some ideas to be realized is very welcome to contribute.

@aldas aldas reopened this Jun 20, 2021
@aldas aldas pinned this issue Jun 20, 2021
@aldas
Copy link
Contributor

aldas commented Jun 20, 2021

Reopening this one and pinning on the top.

If v5 is to be released it will be most probably maintenance release to clean up APIs (remove and/or maybe move things around, fix internally maintenance painpoints). No new fancy features or performance gains/regressions - just annoying API changes for the unprepared :)
If you are not starting with new application you do not need to upgrade to v5.
We probably should support both v4 and v5 for some time (ala 1year) so current users would not be affected by our "busywork" with APIs. From maintainers perspective it seems doable as we do not have (huge/many) changes often. And from library/framework user perspective it should be positive as you are not forced/rushed to upgrade without actual benefits for you.

p.s. most popular thing - logger will be trimmed down to bare minimum as in handlers developer is free to choose what ever he/she chooses to use as the logger instance. also there are few Echo internal places where things are logged and having huge interface for that purpose is unnecessary.

p.s.s. some of the things @vishr mentioned in first post and some other things are already introduced in v4 with minor releases.

@aldas
Copy link
Contributor

aldas commented Oct 2, 2021

Everyone interested, I'll close this issue.

Please use #2000 for discussion of v5 features. I'm sure most of the questions have some form answers there.

@aldas aldas closed this as completed Oct 2, 2021
@aldas aldas unpinned this issue Oct 2, 2021
@aldas
Copy link
Contributor

aldas commented Oct 5, 2021

Anyone who is interested what is planned in v5 and want to share their ideas - please see this discussion #2000

This is announcement and proposed timeline for v5 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests