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

Fix issue 1125 - BodyParam were broken with default values #1129

Merged
merged 2 commits into from Jun 10, 2015

Conversation

Geod24
Copy link
Contributor

@Geod24 Geod24 commented Jun 10, 2015

I have no idea how it used to compile...

@s-ludwig
Copy link
Member

Looks good. New DUB release is needed to get rid of that error.

s-ludwig added a commit that referenced this pull request Jun 10, 2015
Fix issue 1125 - BodyParam were broken with default values
@s-ludwig s-ludwig merged commit 2c3edfc into vibe-d:master Jun 10, 2015
@Geod24
Copy link
Contributor Author

Geod24 commented Jun 10, 2015

Thanks !

Looks good. New DUB release is needed to get rid of that error.

Any ETA ? Also any plan for a Vibe.d release ?

@s-ludwig
Copy link
Member

I would like to wait for dlang/dub#582 to get reviewed/merged, but if that doesn't happen in the near term, I'd just start the release process based on the current status (there are mostly only bug fixes since 0.9.23).

The next vibe.d release should again happen in sync with DMD 2.068. The only thing I still want to review/finish is the start of the transition to using scope for HTTP request handlers, the next big changes, such as the HTTP/2 support will be something for the 0.7.25 release (maybe it makes sense to call it 0.8.0 for a better marketing effect).

@etcimon
Copy link
Contributor

etcimon commented Jun 10, 2015

The only thing I still want to review/finish is the start of the transition to using scope for HTTP request handlers, the next big changes, such as the HTTP/2 support will be something for the 0.7.25 release (maybe it makes sense to call it 0.8.0 for a better marketing effect).

Again many unmerged changes on my fork, I tested everything 1 million times, for leaks as well. Seems good now.

A new Task debugging feature is there as well on

etcimon@d8006c2#diff-ab96042d1071a2f6d25b2d780277a06bR37

See screenshot: http://i.imgur.com/vpVcDnN.jpg

@etcimon
Copy link
Contributor

etcimon commented Jun 10, 2015

As a side note, I wrote this at first as a tool to find leaks, but I found it to be a nice solution for many other things because the link time in Windows is absolutely horrible in --build=debug and I had no symbols in --build=plain, so the errors get thrown with a nice call stack that I can push/pop elements automatically with mixin(Trace);:

etcimon@1951cf2#diff-5a483448dcc1a27cb132021d682e1d1bR279

I think the debugger could gradually become a sort of process manager with debug information, I intend on making a nice GUI javascript framework for it.

@Geod24
Copy link
Contributor Author

Geod24 commented Jun 10, 2015

@s-ludwig : A release of dub soonish would be awesome, because until then, vibe.d master will be red, and that kinda defeat the purpose of CI :)

@Geod24 Geod24 deleted the fix-1125 branch June 10, 2015 21:18
@s-ludwig
Copy link
Member

We can add a quick workaround to travis.yml in the meantime (first do dub upgrade --missing-only and then use --nodeps).

@etcimon I actually have a graphical debugger in the works (didn't have time to work on it since a while though). We should definitely try to at least avoid duplicate work here.

redebug

s-ludwig added a commit that referenced this pull request Jun 11, 2015
@s-ludwig
Copy link
Member

Turns out that the package description of the new test was actually wrong. Should hopefully go green now.

@etcimon
Copy link
Contributor

etcimon commented Jun 11, 2015

@etcimon I actually have a graphical debugger in the works (didn't have time to work on it since a while though). We should definitely try to at least avoid duplicate work here.

It shouldn't be duplicate work, I didn't know you did that btw.

My problem is, I currently find it very hard to write and debug new pages because of all the side effects involved in a complex web application. I get 16mb logs for a few requests, so I'm working on a fiber filtering logging feature. I also don't really have any way to know if fibers have hung or where they hung. It's also a pain to build with symbols all the time the windows linker starts over because of it.

So I actually want to make it a JSON API sort of "task process manager", I have the memory usage per task now, and I can "interrupt" hung tasks and see where they hung even on release builds.

The most important feature for me is the "Recording with filters" feature I'm working on right now. It's a little tiring to have to use log files for each task indiscriminately to examine one little thing

e.g. When you request:
http://localhost:8080/record_once/trace,headers,json_code,category1,category2/the/path/to/debug/goes/here

The page will simply wait for a request to http://localhost:8080/the/path/to/debug/goes/here to complete, and will return when it has captured whatever was declared using the following syntax:
mixin(IfRecording!("headers", "req.headers")) or mixin(IfRecording!("json_code", "mystruct.serializeToJsonString()")), for the task that has the specific breadcrumbs that match this request.

If trace was specified, the mixin(Trace) will be part of the report. I need it to work on release builds as well, it would have to be a debug-for-production tool (very important).

I think the two tools can actually be merged in some way since this will be an API.

@s-ludwig
Copy link
Member

The API level (and below) is where I expected the duplication to happen. What I have is a small library that hooks into the vibe.d application using the Logger interface, as well as setTaskEventCallback. It uses a custom stack tracer for high performace tracing and then uses a binary protocol to talk to the GUI application for efficiency reasons. On top of that it provides some functions to trigger manual debug events or record certain values.

If that would be interesting for you, I could make this part of the project public. It's just a smallish 300 loc library plus the stack trace code.

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

Successfully merging this pull request may close these issues.

None yet

3 participants