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

feat(util:context) add error logging to context #108

Merged
merged 1 commit into from
Sep 25, 2014
Merged

Conversation

btc
Copy link
Contributor

@btc btc commented Sep 23, 2014

@jbenet
@whyrusleeping

This commit...

is a second attempt at: #68

partially addresses: #66

is the result of discussion at: https://gist.github.com/perfmode/f2951c1ed3a02c484d0b

@btc btc added the status/in-progress In progress label Sep 23, 2014
@btc
Copy link
Contributor Author

btc commented Sep 23, 2014

I held off on adding...

func Errors(ctx context.Context) chan<- error {
    v := ctx.Value(ipfsContextErrorLog)
    ch, ok := v.(chan<- error)
    if !ok {
        return nil
    }
    return ch
}

One concern with this is that it leaks the implementation of the error logging. If the parent context was created using context.Background(), then callers may push errors to a nil channel.

@jbenet

@jbenet
Copy link
Member

jbenet commented Sep 23, 2014

@perfmode yep, good point. i guess this is why WithCancel only gives you the cancel func once. LGTM

This commit...

is a second attempt at: #68

partially addresses: #66

is the result of discussion at:

    https://gist.github.com/perfmode/f2951c1ed3a02c484d0b
@btc
Copy link
Contributor Author

btc commented Sep 24, 2014

@whyrusleeping ?

@whyrusleeping
Copy link
Member

Other than an impressively long test name, LGTM

@btc btc merged commit 63a2855 into master Sep 25, 2014
@btc btc removed the codereview label Sep 25, 2014
@btc btc deleted the ipfs-context-2 branch September 25, 2014 03:38
@jbenet
Copy link
Member

jbenet commented Sep 25, 2014

wooo. unlike this:

@btc btc removed their assignment Mar 30, 2015
@aschmahmann aschmahmann mentioned this pull request Feb 18, 2021
73 tasks
ariescodescream pushed a commit to ariescodescream/go-ipfs that referenced this pull request Oct 23, 2021
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.

3 participants