Provide more meaningful error message when adding SSH keys fails

Gitea version (or commit ref): 1.4.1
Git version: 2.16.2
Operating system: Windows Server 2016
Database (use [x]):

[ ] PostgreSQL
[ ] MySQL
[ ] MSSQL
[x] SQLite

Can you reproduce the bug at https://try.gitea.io:

[x] Yes (provide example URL)
[ ] No
[ ] Not relevant

Log gist:

Description
When trying to add a SSH key to a user profile, any type of failure with the ssh-keygen command returns a generic HTTP 500 error to the UI (screenshot attached). In the log you can see what the actual error was, but none of this info is conveyed to the end user.
Reason: Path to ssh-keygen not configured correctly
2018/05/16 17:00:23 [...ters/user/setting.go:430 SettingsKeysPost()] [E] AddPublicKey: 'ssh-keygen -lf C:\Windows\TEMP\gitea_keytest659272622' failed with error 'exec: "ssh-keygen": executable file not found in %PATH%':
Reason: SSH key is not a valid public key - OR - the key is password protected
2018/05/16 17:04:14 [...ters/user/setting.go:430 SettingsKeysPost()] [E] AddPublicKey: 'ssh-keygen -lf C:\Windows\TEMP\gitea_keytest763165203' failed with error 'exec(4:AddPublicKey) failed: exit status 255(<nil>) stdout:  stderr: C:\\Windows\\TEMP\\gitea_keytest763165203 is not a public key file. ': C:\\Windows\\TEMP\\gitea_keytest763165203 is not a public key file.
It would be helpful to have a better error message returned to the user instead of a generic HTTP 500 error (e.g. "Invalid public key or key is password protected", "Path to ssh-keygen is invalid. Contact administrator", etc.)
Screenshots

I agree that level of detail should not be in the user's UI, but even a general error would be better than generic 500 (e.g. "Invalid key", "Invalid SSH configuration. Contact administrator"). That level of detail isn't a security issue and provides a better user experience.

Reason: SSH key is not a valid public key - OR - the key is password protected

@wcarson, can you please explain how a public key can be password protected? Isn't this a typo?
Also, please note that you can get the error is not a public key file from OpenSSH executables even when the key is a perfectly valid key file that just doesn't meet a hard-coded key size limit since OpenSSH version 7.6! See bug 3127 I've just created in their bugtracker.

Both the error message should not be displayed in the user's UI for security reason.

@lunny, can you please elaborate on this? What exactly is wrong on telling the user the key he uploads is not valid or accepted by the server?

@pbodnar By policy we never allow the users to get more information than they need about the system, like paths, versions, etc. What is acceptable is to make our best effort to parse the message and present the user with a sanitized version of it, but that's complicated because tools might change their message format in different versions, or under different locales.

@guillep2k, thank you for the clarification. So is it OK to display a meaningful message at least in the situation when you are actually able to call the ssh-keygen and it returns an error? Or is it too hard to recognize that in Go application?
If one looks at Github as the inspiration, it returns this message: "Key is invalid. You must supply a key in OpenSSH public key format". Moreover, it also displays a hint in the key field that is surely helpful for newbies who don't necessarily always choose the right format for upload.

Hmm... I actually think this is already fixed.
The first case is a server configuration issue for which a 500: Internal Server Error is completely reasonable. The second is already handled I think.
@pbodnar have you actually been able to replicate this issue?

@zeripath, yes, the issue is still reproducible - you can try it for yourself on https://try.gitea.io/, which I suppose is the latest testable Gitea version. Here are the various inputs tested:

Supply a valid key of length at least 1024 bits -> accepted.
Supply a (syntactically) valid key of length of 1023 bits (or possibly shorter) -> still returns error 500. As explained above, you should see the misleading is not a public key file error in the logs.
Supply an invalid key, e. g. supply an invalid algorithm name or change a letter in the encoded key -> correctly returns appropriate errors:
3.1 Can not verify your SSH key: key type and content does not match: ssh-rsb - ssh-rsa
3.2 Can not verify your SSH key: extractTypeFromBase64Key: invalid key format: not enough length 263

Ok so mostly it is fixed - just this second case. Could you provide the logs for the second case.

@zeripath, there is actually nothing interesting logged apart from the error itself that is still nearly the same as in 2018, here it goes (paths shortened):
2020/03/13 08:00:19 ...user/setting/keys.go:96:KeysPost() [E] AddPublicKey: calcFingerprintSSHKeygen: 'ssh-keygen -lf C:\Temp\gitea_keytest395065083' failed with error 'exec(8:AddPublicKey) failed: exit status 255(<nil>) stdout:  stderr: C:\\Temp\\gitea_keytest395065083 is not a public key file.\01503d         ': C:\\Temp\\gitea_keytest395065083 is not a public key file.\01503d


Feature/45 include filter helpers

:tada: This PR is included in version 2.6.0-beta.2 :tada:
The release is available on:

npm package (@beta dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


Fix node_depth

Node depth was not working correctly for graphs with the following structure (decompose node in FEDOT)

Also update_node was not working correctly because only adding node to self._nodes does not add nodes predecessors to graph.nodes

Codecov Report

Merging #152 (f46f1a7) into main (9f8dfcd) will decrease coverage by 0.11%.
The diff coverage is 96.66%.

@@            Coverage Diff             @@
##             main     #152      +/-   ##
==========================================
- Coverage   73.84%   73.73%   -0.11%
==========================================
  Files         129      129
  Lines        7634     7646      +12
==========================================
+ Hits         5637     5638       +1
- Misses       1997     2008      +11

Impacted Files
Coverage Δ

golem/core/dag/graph_utils.py
97.29% <96.55%> (-0.62%)
:arrow_down:

golem/core/dag/linked_graph.py
98.61% <100.00%> (-0.01%)
:arrow_down:

... and 9 files with indirect coverage changes


3dtiles with draco , 1.83 good to view, but newer version failed to view,      !!!waiting your reply!!!

Sandcastle example:
Browser:
Operating System:

dracoGlb.zip
3dtiles with draco , 1.83 good, but newer version failed to view,
I can upload the data to ion, but failed to provide an sandcastle exapmle,  I guess ion can not process glb with draco is the reason,
so please just download the zip file,  it contains a tileset.json and a glb.    you can easily reproduce the problem  I encountered.

Hi @geo3data,
There was a major change to how we handle 3D Tiles in 1.97. Does this tileset work before or after version 1.97?

I can upload the data to ion, but failed to provide an sandcastle example

Cesium ion uses CesiumJS to render the examples. If there is an error loading the file, then the Sandcastle example.

Hi @geo3data,
There was a major change to how we handle 3D Tiles in 1.97. Does this tileset work before or after version 1.97?

I can upload the data to ion, but failed to provide an sandcastle example

Cesium ion uses CesiumJS to render the examples. If there is an error loading the file, then the Sandcastle example.

1.90 Good,  1.93 crash, 1.95 crash,  with  [.WebGL-00000A6402D65400] GL_INVALID_OPERATION: Insufficient buffer size.

I would suggest looking through CHANGES.md for the releases from 1.83 to 1.93 to identify the relevant change.
Perhaps https://github.com/CesiumGS/cesium/issues/9847?

Finally, I found the problem in my code,  after draco, mesh vertex number and triangle count all changed, but I did not notice this, I thought they were not changed,  so close this issue.


Add telemetry data points for gathered notebooks

[x] Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
[x] Title summarizes what is changing.
[x] ~Has a news entry file (remember to thank yourself!).~
[x] Appropriate comments and documentation strings in the code.
[x] Has sufficient logging.
[x] Has telemetry for enhancements.
[x] ~Unit tests & system/integration tests are added/updated.~
[x] ~Test plan is updated as appropriate.~
[x] ~package-lock.json has been regenerated by running npm install (if dependencies have changed).~
[x] ~The wiki is updated with any design decisions/details.~

Codecov Report

Merging #10488 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #10488      +/-   ##
==========================================
- Coverage   60.86%   60.84%   -0.02%
==========================================
  Files         579      579
  Lines       31290    31291       +1
  Branches     4449     4449
==========================================
- Hits        19045    19040       -5
- Misses      11282    11286       +4
- Partials      963      965       +2

Impacted Files
Coverage Δ

src/client/telemetry/index.ts
87.38% <ø> (ø)
:arrow_up:

src/client/datascience/constants.ts
99.71% <100%> (ø)
:arrow_up:

src/datascience-ui/react-common/arePathsSame.ts
75% <0%> (-12.5%)
:arrow_down:

src/client/common/utils/platform.ts
64.7% <0%> (-11.77%)
:arrow_down:

src/client/linters/pydocstyle.ts
86.66% <0%> (-2.23%)
:arrow_down:

src/client/datascience/debugLocationTracker.ts
76.56% <0%> (-1.57%)
:arrow_down:

src/client/common/process/proc.ts
14.49% <0%> (-0.73%)
:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1b2204...82a838e. Read the comment docs.


fix: allow semver ranges

What does this PR do?
Support semver ranges when verifying signature (e.g. sf plugins install<EMAIL_ADDRESS>What issues does this PR fix or reference?
@W-11771849@

QA notes:
✅ install using semver range
before
➜  plugin-trust git:(mdonnalley/semver-range) sf plugins install<EMAIL_ADDRESS>    InvalidNpmMetadata: The npm metadata for plugin plugin-org is missing the versions attribute.
    Code: InvalidNpmMetadata

after
➜  plugin-trust git:(mdonnalley/semver-range) sf plugins install<EMAIL_ADDRESS> ›   Warning: @salesforce/plugin-trust is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
Polling for new version(s) to become available on npm... done
Successfully validated digital signature for @salesforce/plugin-org.
Finished digital signature check.
Installing plugin<EMAIL_ADDRESS>installed v3.3.10

Successfully installed @salesforce/plugin-org v3.3.10

v3.3.10 satisfies ^3.3.6.
and
sf plugins install @salesforce/plugin-org@^2

gets
Successfully installed @salesforce/plugin-org v2.11.8

✅ install using dist-tag
➜  plugin-trust git:(mdonnalley/semver-range) sf plugins install @salesforce/plugin-deploy-retrieve@beta
 ›   Warning: @salesforce/plugin-trust is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
Polling for new version(s) to become available on npm... done
Successfully validated digital signature for @salesforce/plugin-deploy-retrieve.
Finished digital signature check.
Installing plugin<EMAIL_ADDRESS>installed v1.8.8-beta.2
 ›   Warning: @salesforce/plugin-deploy-retrieve > @salesforce/source-deploy-retrieve > proxy-agent > pac-proxy-agent > pac-resolver > degenerator ><EMAIL_ADDRESS>The
 ›   library contains critical security issues and should not be used for production! The maintenance of the project has been discontinued. Consider migrating your code
 ›   to isolated-vm.
 ›   Warning: "@salesforce/plugin-deploy-retrieve > @oclif/core ><EMAIL_ADDRESS>has unmet peer dependency "@types/node@*".
 ›   Warning: "@salesforce/plugin-deploy-retrieve > @oclif/core ><EMAIL_ADDRESS>has unmet peer dependency<EMAIL_ADDRESS>
These warnings can only be addressed by the owner(s) of @salesforce/plugin-deploy-retrieve.
We suggest that you create an issue at https://github.com/forcedotcom/cli/issues and ask the plugin owners to address them.

Successfully installed @salesforce/plugin-deploy-retrieve v1.8.8-beta.2

✅ install specific version:
➜  plugin-trust git:(mdonnalley/semver-range) sf plugins install<EMAIL_ADDRESS> ›   Warning: @salesforce/plugin-trust is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
Skipping digital signature verification because [@cristiand391/sf-plugin-api] is allow-listed.
Finished digital signature check.
Installing plugin<EMAIL_ADDRESS>installed v0.0.2
 ›   Warning: "@cristiand391/sf-plugin-api > @oclif/core ><EMAIL_ADDRESS>has unmet peer dependency "@types/node@*".
 ›   Warning: "@cristiand391/sf-plugin-api > @oclif/core ><EMAIL_ADDRESS>has unmet peer dependency<EMAIL_ADDRESS>
These warnings can only be addressed by the owner(s) of @cristiand391/sf-plugin-api.
We suggest that you create an issue at https://github.com/cristiand391/sf-plugin-api/issues and ask the plugin owners to address them.

Successfully installed @cristiand391/sf-plugin-api v0.0.2

all checks also verified by running sf plugins to confirm the installed version.


Review: Output as separate images for 'sequence of stills' instead of video

Is your feature request related to a problem? Please describe.
When generating a reviewable output, like e.g. reviews from renders or reviews from playblasting in Maya it appears to be impossible to treat the output as a sequence of stills. (Or we couldn't find the setting/feature at least).
So basically a reviewable could output any of:

A video file
An image (a still)
A sequence of images (multiple stills)

Use case
We sometimes produce multiple poses or stills in a single Maya scene to be able to quickly set up and render say 20 poses for a character. However, with the current review system the generated previews will always be a video file uploaded to e.g. Ftrack, etc.
Similarly a single frame reviewable would become a pretty insanely quickly looping video attachment on Ftrack.
Describe the solution you'd like
It's hard to figure out exactly what would be the perfect solution. Becuase technically reviewing a video file and going frame by frame to e.g. draw notes over the poses would work fine (and maybe even better than reviewing 20 stills in some applications).
However, it tends to become a slightly different story if those reviewables also need to pass the client's eyes since they are expecting to see stills and not a video file of 20 frames.
Describe alternatives you've considered
An alternative would be to set up separate instances where each instance is set to a single frame so they are considered to be just the image. Or to ignore that it's a video file and maybe share extracts of that for deliveries (or find the image output of e.g. a playblast and share that instead from disk)
[cuID:861mhk7p3]

So the attribute option Keep Images on the objectset does not give you the functionality you need?

That would at least publish and store the separate images for a playblast in Maya, yes. (Does that also make its way to Ftrack? I'm not sure.)
But in this case I'm also talking about having colorspace corrected review output from e.g. playblasts but also renderlayers as images that would potentially also upload as images to Ftrack.

Does that also make its way to Ftrack? I'm not sure

Not the image themselves, only the review.

but also renderlayers as images that would potentially also upload as images to Ftrack

Are you not able to have a single frame render? Renders should get to Ftrack, but maybe its an issue with it being a movie at that point (?)

Are you not able to have a single frame render? Renders should get to Ftrack, but maybe its an issue with it being a movie at that point (?)

Yes, the issue was mostly with it being a video. It's not a showstopper, but for e.g. a single frame it's definitely confusing that it's a video file both on Ftrack and offline.
Maybe the issue is somewhat related to also easily being able to ship a reviewable to a client - but maybe I'm mixing it too much with what's shown on Ftrack and what gets published.

Marking irrelevant with transition to AYON - feel free to reopen on relevant repo there if problematic issues arise nonetheless. (They are still videos there, but less problematic for us)


fix: remove notification from state on open

Related issues

Fixes #785

Context
Remove notification from state when clicking on it.

I'm trying to test add a test that would like like this but it doesn't work:
  describe('removeNotificationFromState', () => {
    it('should remove a notification from state', async () => {
      const { result } = renderHook(() => useNotifications(false));
      result.current.notifications = mockedSingleAccountNotifications;
      result.current.removeNotificationFromState(mockedSingleAccountNotifications[0].notifications[0].id, mockedSingleAccountNotifications[0].hostname);

      expect(result.current.notifications[0].notifications.length).toBe(0);
    });
  });

I'm not very familiar with React so I'd appreciate some help 😬

Coverage has now increased 🚀


Documentation on performance of compoxure

We were experiencing a lot of timeout issues with services and compoxure felt really slow. I just found that upping the UV_THREADPOOL_SIZE variable really speeds things up a lot. I guess it would be a good idea to document this, and of course I'm now wondering if there are other things that can speed up compoxure as well?

That's very interesting.  We did a whole lot of load testing a ~year ago when we had a memory leak:
<EMAIL_ADDRESS>And there is a simple benchmark in the example folder:
https://github.com/tes/compoxure/blob/master/example/benchmark.js
But not documented - we should definitely approach this more rigorously and document / encapsulate some of the best practice config as a result.


how to detect front or back of hand? given sometimes hand is labeled as wrong left/right hand

Thanks

Hi @xukaiquan, Could you please provide a video  specific to the above wrong observation as mentioned in the issue title. Thanks!


What's the difference between --no-restage and --bail?

From the documentation in README they sound like the same thing.
Is there any scenario where someone would use them together?
Thanks

--bail will abort the commit, --no-restage will not (it just won't re-stage the file in git).

Got it, thanks.

Is there any scenario where someone would use them together or does bail always include no-restage?


Need to be able to define default values for fields and have them in the database and queries

The following can have default values specified in the matter (to be used of the field is missing):

Attributes (default values already supported in ZAP, but this is about getting the spec defined default, not the one in the ZAP config, because this default needs to be usable by the client side of the cluster).
Command fields.
Struct fields.
Event fields.

There is no place for these default values in the database schema, as far as I can see, so no way to use them in templates to codegen code that initializes things to defaults (say).
@mrjerryjohns

So to be clear, for attributes I'd want a default value to be available when doing zcl_attributes.  It looks like that does work right now, actually, so we only need defaults added for command fields, struct fields, and event fields.


[release-1.12] Going to logback 1.4.14

Proposed Changes

as per title

Release Note

Docs

/retest

/retest

/retest-required

/test reconciler-tests

/retest

/test reconciler-tests

/retest


increase backoff deadline and maximum backoff

This change is aimed at addressing rate limiting errors seen during 1000 node cluster testing. Further confirmation is required to confirm this change addresses the issue at scale.

[x] Tests pass
[x] Appropriate changes to documentation are included in the PR

@jdnurme is this PR ready to merge?


The secrets configured in tfaction.yaml are not being set as environment variables by export-secrets.

tfaction version
v1.4.0
Overview
The secrets configured in tfaction.yaml are not being set as environment variables by export-secrets.
However, when the same secrets configuration is set in tfaction-root.yaml, it works properly.
How to reproduce
tfaction-root.yaml
plan_workflow_name: test
target_groups:
- working_directory: terraform/
  target: terraform/dev/
  aws_region: ap-northeast-1
  s3_bucket_name_tfmigrate_history: tfaction-************
  terraform_plan_config:
    aws_assume_role_arn: arn:aws:iam::************:role/tfaction
  tfmigrate_plan_config:
    aws_assume_role_arn: arn:aws:iam::************:role/tfaction
  terraform_apply_config:
    aws_assume_role_arn: arn:aws:iam::************:role/tfaction
  tfmigrate_apply_config:
    aws_assume_role_arn: arn:aws:iam::************:role/tfaction

tfaction.yaml
secrets:
- env_name: NEW_RELIC_API_KEY
  secret_name: NEWRELIC_API_KEY_DEV

GitHub Actions Workflow
I have created a workflow based on this example as a reference.
I am using secrets: inherit to pass secrets to the reusable workflow.
--- test.yaml
name: pull_request_target
on: pull_request_target

concurrency:
  group: ${{ github.workflow }}--${{ github.head_ref }} # github.ref is unavailable in case of pull_request_target
  cancel-in-progress: true

permissions: {}

jobs:
  path-filter:
    uses: ./.github/workflows/wc-path-filter.yaml
    permissions: {}
    secrets:
      gh_app_id: ${{secrets.TFACTION_GITHUB_APP_ID}}
      gh_app_private_key: ${{secrets.TFACTION_GITHUB_APP_PRIVATE_KEY}}

  enable-auto-merge:
    # This job is used for main branch's branch protection rule's status check.
    # If all dependent jobs succeed or are skipped this job succeeds.
    uses: ./.github/workflows/wc-enable-auto-merge.yaml
    needs:
      - status-check
    permissions: {}
    secrets:
      gh_app_id: ${{secrets.TFACTION_GITHUB_APP_ID}}
      gh_app_private_key: ${{secrets.TFACTION_GITHUB_APP_PRIVATE_KEY}}
    if: |
      ! failure() && ! cancelled() && github.event.pull_request.user.login == 'renovate[bot]' && contains(github.event.pull_request.body, ' **Automerge**: Enabled.')

  # This job is used for branch protection rule
  # Add this job to `Status checks that are required`
  status-check:
    runs-on:
      - self-hosted
      - dev
    needs:
      - test
    if: failure()
    steps:
      - run: exit 1

  test:
    uses: ./.github/workflows/wc-test.yaml
    needs: path-filter
    permissions:
      id-token: write
      contents: read
    with:
      ref: ${{needs.path-filter.outputs.merge_commit_sha}}
    secrets: inherit

--- wc-test.yaml
name: test
on:
  workflow_call:
    inputs:
      ref:
        required: false
        type: string

env:
  TFACTION_IS_APPLY: 'false'
  GH_COMMENT_SHA1: ${{inputs.ref}}
  TFCMT_SHA: ${{inputs.ref}}

permissions: {}

jobs:
  setup:
    uses: ./.github/workflows/wc-setup.yaml
    with:
      ref: ${{inputs.ref}}
    secrets:
      gh_app_id: ${{secrets.TFACTION_GITHUB_APP_ID}}
      gh_app_private_key: ${{secrets.TFACTION_GITHUB_APP_PRIVATE_KEY}}
    permissions:
      contents: read

  plan:
    uses: ./.github/workflows/wc-plan.yaml
    needs: setup
    # if services is empty, the build job is skipped
    if: join(fromJSON(needs.setup.outputs.targets), '') != ''
    with:
      targets: ${{needs.setup.outputs.targets}}
      ref: ${{inputs.ref}}
    secrets: inherit
    permissions:
      id-token: write
      contents: read

--- wc-plan.yaml
name: plan
on:
  workflow_call:
    inputs:
      targets:
        required: true
        type: string
      ref:
        required: true
        type: string

jobs:
  plan:
    name: "plan (${{matrix.target.target}})"
    runs-on: ${{matrix.target.runs_on}}
    permissions:
      id-token: write # For OIDC
      contents: read # To checkout private repositories
    env:
      TFACTION_TARGET: ${{matrix.target.target}}
      TFACTION_JOB_TYPE: ${{matrix.target.job_type}}
      GH_COMMENT_SHA1: ${{inputs.ref}}
      TFCMT_SHA: ${{inputs.ref}}
    strategy:
      fail-fast: true
      matrix:
        target: ${{fromJSON(inputs.targets)}}
    steps:
      - uses: actions/checkout@1d96c772d19495a3b5c517cd2bc0cb401ea0529f # v4.1.3
        with:
          ref: ${{inputs.ref}}

      - name: Generate token (aqua)
        id: aqua_installer_token
        uses: tibdex/github-app-token@3beb63f4bd073e61482598c45c71c1019b59b73a # v2.1.0
        with:
          app_id: ${{secrets.TFACTION_GITHUB_APP_ID}}
          private_key: ${{secrets.TFACTION_GITHUB_APP_PRIVATE_KEY}}
          permissions: >-
            {}
          repositories: >-
            []

      - uses: aquaproj/aqua-installer@4551ec64e21bf0f557c2525135ff0bd2cba40ec7 # v3.0.0
        with:
          aqua_version: v2.27.3
        env:
          AQUA_GITHUB_TOKEN: ${{steps.aqua_installer_token.outputs.token}}

      - uses: suzuki-shunsuke/tfaction/export-secrets@c56c96f8587d75ec918c79256b1f9c50361287fd # v1.4.0
        with:
          # If this action doesn't export secrets as you expect, please check if secrets are passed to this action properly.
          # If you use reusable workflows, maybe secrets aren't passed to the reusable workflow.
          # If so, please pass secrets properly.
          #
          # - https://docs.github.com/en/actions/using-workflows/reusing-workflows#passing-inputs-and-secrets-to-a-reusable-workflow
          # - https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idsecrets
          secrets: ${{toJSON(secrets)}}

      - name: debug
        run: |
          echo ${NEW_RELIC_API_KEY}

      - name: Generate token to download private Terraform Modules
        id: gh_setup_token
        uses: tibdex/github-app-token@3beb63f4bd073e61482598c45c71c1019b59b73a # v2.1.0
        with:
          app_id: ${{ secrets.TFACTION_GITHUB_APP_ID }}
          private_key: ${{ secrets.TFACTION_GITHUB_APP_PRIVATE_KEY }}
          # If you use private registries, contents:read is required
          permissions: >-
            {
              "contents": "read"
            }
          # private repositories hosting private modules
          repositories: >-
            []

      # This is required to download private modules in `terraform init`
      - run: gh auth setup-git
        env:
          GITHUB_TOKEN: ${{steps.gh_setup_token.outputs.token}}

      - name: Generate token
        id: generate_token
        uses: tibdex/github-app-token@3beb63f4bd073e61482598c45c71c1019b59b73a # v2.1.0
        with:
          app_id: ${{secrets.TFACTION_GITHUB_APP_ID}}
          private_key: ${{secrets.TFACTION_GITHUB_APP_PRIVATE_KEY}}
          # issues:write - Create labels
          # pull_requests:write - Post comments and set labels
          # contents:write - Push commits
          permissions: >-
            {
              "pull_requests": "write",
              "issues": "write",
              "contents": "write"
            }
          repositories: >-
            ["${{github.event.repository.name}}"]

      - uses: suzuki-shunsuke/tfaction/setup@c56c96f8587d75ec918c79256b1f9c50361287fd # v1.4.0
        with:
          github_token: ${{steps.generate_token.outputs.token}}
        env:
          GITHUB_TOKEN: ${{steps.generate_token.outputs.token}} # For GitHub Provider

      - uses: suzuki-shunsuke/tfaction/get-target-config@c56c96f8587d75ec918c79256b1f9c50361287fd # v1.4.0
        id: target-config

      - name: Generate token (pull request write)
        id: pull_request_token
        uses: tibdex/github-app-token@3beb63f4bd073e61482598c45c71c1019b59b73a # v2.1.0
        with:
          app_id: ${{secrets.TFACTION_GITHUB_APP_ID}}
          private_key: ${{secrets.TFACTION_GITHUB_APP_PRIVATE_KEY}}
          permissions: >-
            {
              "pull_requests": "write"
            }
          repositories: >-
            ["${{github.event.repository.name}}"]

      - uses: suzuki-shunsuke/tfaction/test@c56c96f8587d75ec918c79256b1f9c50361287fd # v1.4.0
        with:
          github_token: ${{steps.generate_token.outputs.token}}
        env:
          GITHUB_TOKEN: ${{steps.generate_token.outputs.token}} # For GitHub Provider and reviewdog and github-comment

      - uses: suzuki-shunsuke/tfaction/plan@c56c96f8587d75ec918c79256b1f9c50361287fd # v1.4.0
        with:
          github_token: ${{steps.generate_token.outputs.token}}
        env:
          GITHUB_TOKEN: ${{steps.generate_token.outputs.token}} # For GitHub Provider and tfcmt and github-comment

GitHub Actions' log
2024-04-24T09:22:23.9027791Z ##[group]Run suzuki-shunsuke/tfaction/export-secrets@c56c96f8587d75ec918c79256b1f9c50361287fd
2024-04-24T09:22:23.9029200Z with:
2024-04-24T09:22:23.9100101Z   secrets: {
  "NEWRELIC_API_KEY_DEV": "***"
}
2024-04-24T09:22:23.9116554Z env:
2024-04-24T09:22:23.9117384Z   TFACTION_TARGET: terraform/dev/dev
2024-04-24T09:22:23.9118374Z   TFACTION_JOB_TYPE: terraform
2024-04-24T09:22:23.9119441Z   GH_COMMENT_SHA1: 55de72c838e65f04dc7c192c36ccf4bbb6add185
2024-04-24T09:22:23.9120663Z   TFCMT_SHA: 55de72c838e65f04dc7c192c36ccf4bbb6add185
2024-04-24T09:22:23.9121733Z ##[endgroup]
2024-04-24T09:22:24.0047333Z The list of secret names passed to the action: NEWRELIC_USER_KEY_DEV
2024-04-24T09:22:24.0276970Z ##[group]Run echo ${NEW_RELIC_API_KEY}
2024-04-24T09:22:24.0278033Z echo ${NEW_RELIC_API_KEY}
2024-04-24T09:22:24.0334719Z shell: /usr/bin/bash -e {0}
2024-04-24T09:22:24.0335625Z env:
2024-04-24T09:22:24.0336428Z   TFACTION_TARGET: terraform/dev/dev
2024-04-24T09:22:24.0337408Z   TFACTION_JOB_TYPE: terraform
2024-04-24T09:22:24.0338480Z   GH_COMMENT_SHA1: 55de72c838e65f04dc7c192c36ccf4bbb6add185
2024-04-24T09:22:24.0339714Z   TFCMT_SHA: 55de72c838e65f04dc7c192c36ccf4bbb6add185
2024-04-24T09:22:24.0340747Z ##[endgroup]

Expected behaviour
The log states: export the secret NEWRELIC_API_KEY_DEV as the environment variable NEW_RELIC_API_KEY.
Below is the configuration of secrets written in tfaction-root.yaml:
2024-04-24T09:18:02.2130049Z ##[group]Run suzuki-shunsuke/tfaction/export-secrets@c56c96f8587d75ec918c79256b1f9c50361287fd
2024-04-24T09:18:02.2131402Z with:
2024-04-24T09:18:02.2206516Z   secrets: {
  "NEWRELIC_API_KEY_DEV": "***",
}
2024-04-24T09:18:02.2223279Z env:
2024-04-24T09:18:02.2224121Z   TFACTION_TARGET: terraform/dev/dev
2024-04-24T09:18:02.2225146Z   TFACTION_JOB_TYPE: terraform
2024-04-24T09:18:02.2226270Z   GH_COMMENT_SHA1: 5b2974c9ce2d3329b198fe2d9aa5b08b4aa08496
2024-04-24T09:18:02.2227546Z   TFCMT_SHA: 5b2974c9ce2d3329b198fe2d9aa5b08b4aa08496
2024-04-24T09:18:02.2228676Z ##[endgroup]
2024-04-24T09:18:02.3153510Z The list of secret names passed to the action: NEWRELIC_API_KEY_DEV
2024-04-24T09:18:02.3184615Z export the secret NEWRELIC_API_KEY_DEV as the environment variable NEW_RELIC_API_KEY
2024-04-24T09:18:02.3332689Z ##[group]Run echo ${NEW_RELIC_API_KEY}
2024-04-24T09:18:02.3333773Z echo ${NEW_RELIC_API_KEY}
2024-04-24T09:18:02.3392619Z shell: /usr/bin/bash -e {0}
2024-04-24T09:18:02.3393569Z env:
2024-04-24T09:18:02.3394407Z   TFACTION_TARGET: terraform/dev/dev
2024-04-24T09:18:02.3395445Z   TFACTION_JOB_TYPE: terraform
2024-04-24T09:18:02.3396555Z   GH_COMMENT_SHA1: 5b2974c9ce2d3329b198fe2d9aa5b08b4aa08496
2024-04-24T09:18:02.3397855Z   TFCMT_SHA: 5b2974c9ce2d3329b198fe2d9aa5b08b4aa08496
2024-04-24T09:18:02.3399166Z   NEW_RELIC_API_KEY: ***
2024-04-24T09:18:02.3400107Z ##[endgroup]
2024-04-24T09:18:02.3491770Z ***

tfaction-root.yaml
plan_workflow_name: test
target_groups:
- working_directory: terraform/
  target: terraform/dev/
  aws_region: ap-northeast-1
  secrets:
  - env_name: NEW_RELIC_API_KEY
    secret_name: NEWRELIC_API_KEY_DEV
  s3_bucket_name_tfmigrate_history: tfaction-************
  terraform_plan_config:
    aws_assume_role_arn: arn:aws:iam::************:role/tfaction
  tfmigrate_plan_config:
    aws_assume_role_arn: arn:aws:iam::************:role/tfaction
  terraform_apply_config:
    aws_assume_role_arn: arn:aws:iam::************:role/tfaction
  tfmigrate_apply_config:
    aws_assume_role_arn: arn:aws:iam::************:role/tfaction

tfaction.yaml
{}

Actual behaviour
The environment variable NEW_RELIC_API_KEY is not being set.
Important Factoids
This issue seems similar to https://github.com/suzuki-shunsuke/tfaction/issues/1564, where despite the log message in export-secrets stating The list of secret names passed to the action: NEWRELIC_API_KEY_DEV, the environment variable is not being set.
After reviewing the code in the following files:

https://github.com/suzuki-shunsuke/tfaction/blob/8d5c4769af3a845f03d1f3f90b79fb981c849a01/export-secrets/src/index.ts
https://github.com/suzuki-shunsuke/tfaction/blob/8d5c4769af3a845f03d1f3f90b79fb981c849a01/export-secrets/src/lib.ts

I couldn't find the part where it retrieves secrets from tfaction.yaml."
Note
No response

The log looks strange.
2024-04-24T09:22:23.9027791Z ##[group]Run suzuki-shunsuke/tfaction/export-secrets@c56c96f8587d75ec918c79256b1f9c50361287fd
2024-04-24T09:22:23.9029200Z with:
2024-04-24T09:22:23.9100101Z   secrets: {
  "NEWRELIC_API_KEY_DEV": "***"
}

2024-04-24T09:22:24.0047333Z The list of secret names passed to the action: NEWRELIC_USER_KEY_DEV

I'm not sure where did NEWRELIC_USER_KEY_DEV come from. 🤔

I'm getting the same issue
I have set the config in tfaction.yaml

and I passed down the secret properly, It shows up in the logs but its not being set as an env vars.


chore(deps): update roadie scaffolder modules

Description
During all of the recent updates, we realized that a couple of the roadie scaffolder backend modules were broken. This PR updates them after they have been fixed upstream. There was also a disabled test that has now been re-enabled.
Which issue(s) does this PR fix

Fixes RHIDP-4838
Fixes RHIDP-4837

PR acceptance criteria
Please make sure that the following steps are complete:

[ ] GitHub Actions are completed and successful
[ ] Unit Tests are updated and passing
[ ] E2E Tests are updated and passing
[ ] Documentation is updated if necessary (requirement for new features)
[ ] Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

/retest

It looks like the e2e test that I re-enabled is failing again.
Taking a look at the pod logs shows that the roadiehq-scaffolder-backend-module-http-request-dynamic is not being installed even though I have changed it in the values file. Best guess is that the values that are used for the deployment originate from main and not the PR.
Going to re-disable the test and then create a subsequent PR after this is merged to re-re-enable the failing test.

/retest


[KIP] - add a version of #parseKORE that can handle ill-formed input

Motivation
Currently, when #parseKORE is called on a string which is not valid Kore in a semantics compiled using the LLVM backend, the semantics interpreter crashes. This means that we cannot have:

custom error messages for Kore parse errors - we get whatever the hook gives us
custom error handling for Kore parse errors - we crash

Allowing for a custom error handling routine for our tools which parse Kore will make our tooling appear more polished to our clients and make debugging slightly easier.
As an example, we have a tool for a client where we use #unparseKORE to store values in a database and #parseKORE to read them out. Currently, if for any reason, for example, due to WIP code, upgrades, network issues, random bit-flips, etc. the semantics and the database get out of sync then the client will just see something like this:
...
/home/stephen/work/kframework/master/k-distribution/target/release/k/bin/krun: line 70:  2462 Aborted                 "$@"
[Error] Critical: Backend crashed during rewriting with exit code 134

where the ... seems to be the first token encountered which the parser failed to parse. To a non-technical user, this will be confusing at best and give us a bad look at worst.
Example K Code
Ideally, we would like a version of #parseKORE which is a provably total function at compile time. For this new version to work, we have a couple different possible approaches as far as I can see:

provide a default value which matches the inferred target sort

syntax {Sort} Sort ::= #parseKORE(String, Sort) [hook(...)]

// Behavior:
//
// Given:
//
// syntax Int ::= #parseInt(String) [function, functional]
// rule #parseInt(S) => #parseKORE(S, -1)
//
// We expect:
//
// #parseInt("... valid Kore for Int 5 ...") => 5
// #parseInt("foo") => -1

provide a default value and an explicit target sort (may help with debugging):

// NB:
// 1.  the first String argument below represents a K Sort
// 2.  the second String argument below represents the Kore to be parsed
//
syntax {Sort} Sort ::= #parseKORE(String, String, Sort) [hook(...)]

// Behavior:
//
// #parseKORE("Int", "... valid Kore for Int 5 ..", -1) =>  5
// #parseKORE("Int", "foo", -1)                         => -1
// #parseKORE("----", "foo", -1)                              // compile error

return a fresh sort for parse results

syntax KParseResult ::= KParseResult(K)
                      | KParseFailure(String)

// NB:
// 1. the first String argument below represents a K Sort
// 2. the second String argument below represents the Kore to be parsed
//
syntax KParseResult ::= #parseKORE(String, String) [function, hook(...)]

// Behavior:
//
// #parseKORE("Int", "... valid Kore for Int 5 ...") => KParseResult(5)
// #parseKORE("Int", "foo") => KParseFailure("an error message")

Documentation
The #parseKORE family of functions all operate as follows:

they infer their output sort T based on where they occur in the K AST (or perhaps are given an explicit target sort T)
they take a String as input and attempt parse it as Kore representation of some arbitrary T value v
if the parse is successful, the value v is returned (or a KParseResult is returned)

Where the members of this family differ is in how they handle Kore parse errors:

when the function #parseKORE(String) is called with a String that is invalid Kore, it raises a runtime error and interpretation halts
when the function #parseKORE(..., String, ...) is called with a String that is invalid Kore, it returns a default T value (or a KParseFailure is returned)

Potential Alternatives/Work-arounds
This functionality is not required for any semantics that we write --- it is just a quick, easy, and fast way to load data into a K semantics.
We could always write explicit unparsers and parsers in K code do all parsing/unparsing internally and have full control over the error messages and error handling. The downside to this approach is that it is much more work intensive.
Testing Approach
Testing this feature would require a small test files using examples similar to the ones I used above and would have no problems being integrated into the standard K test harness.

I mean, it's unlikely that we're going to be able to get to a point where the parseKORE hook is able to handle /any/ string without problems in any reasonably-near-term time window. The llvm backend doesn't really validate its inputs much, by design. The assumption is that it's the responsibility of the person invoking the backend to pass it a valid KORE term that is well-formed and a valid instance of the sort the backend is expecting. This makes sense, because validating a term is expensive compared to simply constructing it and rewriting it, and generally speaking terms that are inputs are in most cases generated by well-tested parsers that have already been shown to generate well-formed terms. Validating the term would thus significantly increase the overhead of rewriting. I agree that this seems to cause a problem if your use case is that you are storing terms in a database and passing them to a version of the semantics that is different from the one that generated the term. My best advice would be, if you need reasonable backwards-compatible guarnatees about the encoding of terms in the database that are proof against modifications to your semantics, using KORE as the representation of those terms is probably not wise. You are better off creating an explicitly versioned representation of the terms you need to store and writing a decoder which is capable of decoding serialized terms of any version into the correct, modern representation of those terms.

Well, one solution would be to just call the Haskell backend term validator against the term before parsing. The LLVM backend doesn't have to handle this directly, we certainly have existing routines for validating kore terms already.


OLS-617: Add alert runbooks as a RAG content source

OLS-617: Add alert runbooks as a RAG content source
OCP product docs update 2024/06/12
make format fixes

Links
JIRA: https://issues.redhat.com/browse/OLS-617

/test "Red Hat Konflux / lightspeed-rag-content-on-pull-request"

/retest

looks like there are some "deprecated" folders in the runbook content, seems likely we should exclude those?  (i see them under openshift-virtualization-operator, at least)
otherwise this lgtm

looks like there are some "deprecated" folders in the runbook content, seems likely we should exclude those? (i see them under openshift-virtualization-operator, at least)
otherwise this lgtm

Took care of this one, too.

/approve
/lgtm


Add Medtronic serial number field on device selection

This PR adds a serial number field for direct Medtronic device uploads. The serial number field should require no more or fewer than six digits, and should ONLY allow numeric input.
It also adds new metrics when clicking on "Choose devices" on the user menu and when the "Done" button is clicked on the device selection page.
See  Trello card for more info.
@krystophv: mind taking a look when you have time? @gniezen reviewed and merged in his branch from the previous PR (#340), so it'd be nice to have another set of eyes.

The fact that it doesn't emit a change event until something comes after the 'e' is pretty incredible (and actually sounds like a bug). The solution seems pretty robust. LGTM.


Make manga page preview row count configurable

Closes #643
Add a new appearance setting for the manga page preview row count. Allows between 0 (disabled) and 10 rows.
Screenshots

Default (4 rows)
Disabled

I think placing this feature on E-hentai would be better than putting it in fork setting.🤔

Fork settings in appearance is a good spot for it


BAB Buy Now Lag

When clicking Buy Now on the Buy Anything Button, there's a long wait, almost a minute before the "Pay" comes up. Earlier I left the page thinking I'm done, but now I don't, because I anticipate it. I suspect many people are lost in this step.

First try: 30 seconds waiting for server's response. This can happen time to time if the server is slow for some reason, the admin page as well was responding slowly.
https://github.com/zkSNACKs/WalletWasabi/assets/54551879/a2d718eb-3014-43e4-9476-627b00bdb5c8
Second try: Barely 7 seconds, the server was responding faster.
https://github.com/zkSNACKs/WalletWasabi/assets/54551879/c2bfa6b0-b642-493b-ae7c-2c7f05891af1
Third try: About 5 seconds, even faster then before.
https://github.com/zkSNACKs/WalletWasabi/assets/54551879/756c50a9-87c3-4fcf-8430-5a0dc73ef8e8
Not much to do on our side. We could justify in a system chat message that a payment invoice is being made and will be sent ASAP.

There's always a way

BAB is discontinued https://github.com/WalletWasabi/WalletWasabi/pull/13558


[1302] fix authorization errors

https://trello.com/c/OnBKJMq6/1302-authorization-failure-when-updating-site-statuses
Context
Apparently Faraday is not thread-safe and the way we were setting the
authorization token on the connection was opening us up to a race condition.
Changes proposed in this pull request
This plugs that hole by using the Thread key-value context to save the user
token, and using that when we generate the connection to mcb.
Guidance to review
This PR requires #242 and is based off of that one.

 Authorization failure when updating site statuses

@tvararu

Should we throw something up to Sentry if this lookup finds nil?

Added a .fetch. I couldn't think of a way to test it outside of a unit test as I don't see how this situation would occur, so I'm not sure we need to test it.

@tvararu

Should we throw something up to Sentry if this lookup finds nil?

Added a .fetch. I couldn't think of a way to test it outside of a unit test as I don't see how this situation would occur, so I'm not sure we need to test it.

passing thought/no idea
Thread.current[:manage_courses_bac
kend_token] == nil raise hell

?

passing thought/no idea
Thread.current[:manage_courses_bac
kend_token] == nil raise hell

?

This is what .fetch does, it allows a default to be provided, but if not it raises an exception.


Add a prefix to vault mappings

This allows us to ensure that folks can have different secrets for different namespaces

Hi @heroic. Thanks for your PR.
I'm waiting for a jenkins-x or todo member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository.

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by:
To complete the pull request process, please assign rajdavies
You can assign the PR to them by writing /assign @rajdavies in a comment when ready.
The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

/ok-to-test

@heroic: The following test failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

pr
4c25aa20d4eae732984b1892322875e50f54ccbf
link
/retest

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository. I understand the commands that are listed here.

/lgtm

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: jstrachan
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS [jstrachan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment


Span buffering

Spans are buffered according to the following strategy:
On the regular basis, spans will be buffered for a second up to the maximum of options.MaxBufferedSpans, defaulting to 1000. Whenever the span buffer is filled up to options.ForceTransmissionStartingAt (default 500) within this second, transmission will be made.
Leaving any of these options at zero causes default values to be used. Immediate spans transmission doesn't take place any more.

Merging and tagging for a release


Ability to direct Error and Fatal to Console.Error instead of Console.Out

It can be useful for many automation reasons to direct Error and Fatal logs to standard error (Console.Error) or its redirection rather than to standard out.

Thanks for the suggestion, I agree it's a worthwhile option to add. Do you have any proposal for how the API should look? errorsToStderr: true as a configuration arg? Thanks!

That API would work for my current needs. I've also seen apps that send all logs to standard error (as they always expect standard output to be redirected), so I'm not sure if that might also be a useful feature to the next developer and maybe someone might want an API more like minimumLevelToStandardError: LogEventLevel.Error.

minimumLevelToStandardError sounds like a good move, thanks for the suggestion. The name of the arg (starting with m) might catch people's attention when what they're looking for is restrictedToMinimumLevel; perhaps standardErrorFromLevel: or standardErrorFromMinimumLevel:?
We probably should do this across Console() and ColoredConsole() as well.
Marking this one as "up-for-grabs". Cheers!

Closed in #12.


How to expose statistics like "branch misses" in CodeGen?

CodegenPerf.cpp has shown some critical codegen statistics like branch misses and branch instructions. However, I have not seen any such statistics exposed during running CodegenPerf or any other benchmark tests for Velox. May I ask how can I enable exposing such statistics in velox?

@laithsakka has more context on that codebase; could you help @markjin1990 here?

@markjin1990  Hello. We do not have a consistent way to export the statistics you are mentioning on the codegen. The example you are seeing was an adoc measurement we did for experimentation.
We do have an API however to produce the desired statistics (codegen/vector_function/Perf.h), but you would need to insert explicit calls to collect the counter values around the code you are interrested in.
Hope it helps.

Closing this one for now. Please re-open if there are any further questions.


[HUDI-8182]Cache internalSchema for hive read, avoid each split reloa…

…d active timeline.
Change Logs
Addressing this issue: 11723
When using MR to read Hudi, use a global InternalSchema to avoid each reader listing the metadata directory to obtain the InternalSchema, which places a significant load on the HDFS NameNode.
Impact
Reduced metadata listing frequency to alleviate pressure on the HDFS NameNode.
Risk level (write none, low medium or high below)
low
Documentation Update
N/A
Contributor's checklist

[ ] Read through contributor's guide
[ ] Change Logs and Impact were stated clearly
[ ] Adequate tests were added if applicable
[ ] CI passed

@muyihao any update on addressing the comments in the PR?

@muyihao any update on addressing the comments in the PR?

Currently working on verifying whether HoodieCombineHiveInputFormat can handle multiple Hudi tables and making modifications based on the comments.

Hi, @muyihao Is this ready for review now?

Hi, @muyihao Is this ready for review now?

@danny0405 Yes, sorry for the late reply. Thank you for your help with the review. Currently, the modifications cache the latest schema for different tables when there are multiple table paths. However, this does not completely avoid loading the ActiveTimeline. When reading COW, it will still trigger the loading of the active timeline when searching for the fileSchema.

When reading COW, it will still trigger the loading of the active timeline when searching for the fileSchema.

Can you show us the line that triggers this logic?

When reading COW, it will still trigger the loading of the active timeline when searching for the fileSchema.

Can you show us the line that triggers this logic?

InternalSchemaCache#searchSchemaAndCache will trigger InternalSchemaCache#getSchemaByReadingCommitFile or InternalSchemaCache##getHistoricalSchemas, both may load active timeline

@muyihao Thanks for pinning it out, the metaClient.getActiveTimeline also choose to return the cached timeline first, for one metaClient instantce, the list would happen only once, so I guess it is not a problem if the metaClient can be shared for multiple input splits.
BTW, while I reviewing the code, I found a bug for FileBasedInternalSchemaStorageManager, the passed in metaClient does not always work if the base path of it is not end up with .schema, we should fix it like this:
  public FileBasedInternalSchemaStorageManager(HoodieTableMetaClient metaClient) {
    this.baseSchemaPath = new StoragePath(metaClient.getMetaPath(), SCHEMA_NAME);
    this.storage = metaClient.getStorage();
    this.metaClient = metaClient.getBasePath().getName().equalsIgnoreCase(SCHEMA_NAME) ? metaClient : null;
  }

@muyihao Thanks, I have applied a patch again the master code(instead of your patch) to address the timeline list issue, you can merge the patch first then your Job conf set up logic, then it should be working:
8182.patch.zip

@danny0405 Thanks for the help, I have merged it.

The test failures are not related, will merge it.

The test failures are not related, will merge it.

Thanks for helping to land this PR. :)


ContentLength with Body length 0 errors

The common issue so far seems to be apps which set content-length and don't respond with a body, or partial body etc. The errors from the reverse proxy don't make it clear what's happened.

Firstly thanks for the awesome tool ❤
Is this the same as ERRO network error: http: ContentLength=1058 with Body length 0? getting this error alot would love to be able to dig a bit better and find out the offending url etc.

@cloudkite thanks! We were thinking that case is a bug in the proxy, I'll try and reproduce it today. If you have any isolated code where it fails reasonably consistently or notice any patterns let me know!
Do you see any "retrying idempotent request" in the logs?

@tj dont see any "retrying idempotent request" in the logs
This is what I get in the logs
{"fields":{"app":"HelpHero","error":"EOF","plugin":"relay","region":"us-west-2","version":"13"},"level":"error","timestamp":"2017-09-27T05:15:56.263949574Z","message":"network"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"warn","timestamp":"2017-09-27T05:15:56.263992087Z","message":"restarting"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:56.264059763Z","message":"found free port 40065"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:56.264087603Z","message":"executing \"cd dist \u0026\u0026 node-v8.4.0-linux-x64/bin/node lib/functions\""}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:56.266981961Z","message":"proxy (pid=29) started"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:56.267024325Z","message":"waiting for http://<IP_ADDRESS>:40065 to listen (timeout 15s)"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","restarts":1,"version":"13"},"level":"warn","timestamp":"2017-09-27T05:15:57.593718135Z","message":"restarted"}
{"fields":{"app":"HelpHero","error":"http: ContentLength=1058 with Body length 0","plugin":"relay","region":"us-west-2","version":"13"},"level":"error","timestamp":"2017-09-27T05:15:57.594089195Z","message":"network"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"warn","timestamp":"2017-09-27T05:15:57.594116478Z","message":"restarting"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:57.594177518Z","message":"found free port 45525"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:57.594210115Z","message":"executing \"cd dist \u0026\u0026 node-v8.4.0-linux-x64/bin/node lib/functions\""}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:57.604005752Z","message":"proxy (pid=36) started"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:57.604043442Z","message":"waiting for http://<IP_ADDRESS>:45525 to listen (timeout 15s)"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","restarts":2,"version":"13"},"level":"warn","timestamp":"2017-09-27T05:15:58.930635714Z","message":"restarted"}
{"fields":{"app":"HelpHero","error":"http: ContentLength=1058 with Body length 0","plugin":"relay","region":"us-west-2","version":"13"},"level":"error","timestamp":"2017-09-27T05:15:58.930997464Z","message":"network"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"warn","timestamp":"2017-09-27T05:15:58.931024558Z","message":"restarting"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:58.931085031Z","message":"found free port 38441"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:58.931120843Z","message":"executing \"cd dist \u0026\u0026 node-v8.4.0-linux-x64/bin/node lib/functions\""}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:58.931534264Z","message":"proxy (pid=43) started"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T05:15:58.931569487Z","message":"waiting for http://<IP_ADDRESS>:38441 to listen (timeout 15s)"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","restarts":3,"version":"13"},"level":"warn","timestamp":"2017-09-27T05:16:00.253728035Z","message":"restarted"}
{"fields":{"app":"HelpHero","duration":3992,"id":"f0dfd800-a342-11e7-967b-c317544628a5","method":"POST","path":"/api/query","plugin":"logs","query":"trace=nav-startVisit","region":"us-west-2","size":0,"stage":"production","status":502,"version":"13"},"level":"error","timestamp":"2017-09-27T05:16:00.254142004Z","message":"response"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"warn","timestamp":"2017-09-27T05:16:20.168008382Z","message":"proxy (pid=17) sending SIGKILL"}

on some occassion it leads with a  connection reset by peer instead of EOF
{"fields":{"app":"HelpHero","error":"read tcp <IP_ADDRESS>:48090-\u003e<IP_ADDRESS>:39613: read: connection reset by peer","plugin":"relay","region":"us-west-2","version":"13"},"level":"warn","timestamp":"2017-09-27T12:11:24.726339448Z","message":"temporary"}
{"fields":{"app":"HelpHero","error":"http: ContentLength=1058 with Body length 0","plugin":"relay","region":"us-west-2","version":"13"},"level":"error","timestamp":"2017-09-27T12:11:24.826867736Z","message":"network"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"warn","timestamp":"2017-09-27T12:11:24.826921335Z","message":"restarting"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T12:11:24.826982667Z","message":"found free port 45465"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T12:11:24.827018379Z","message":"executing \"cd dist \u0026\u0026 node-v8.4.0-linux-x64/bin/node lib/functions\""}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T12:11:24.844072155Z","message":"proxy (pid=54) started"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T12:11:24.84412175Z","message":"waiting for http://<IP_ADDRESS>:45465 to listen (timeout 15s)"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","restarts":4,"version":"13"},"level":"warn","timestamp":"2017-09-27T12:11:26.173694539Z","message":"restarted"}
{"fields":{"app":"HelpHero","error":"http: ContentLength=1058 with Body length 0","plugin":"relay","region":"us-west-2","version":"13"},"level":"error","timestamp":"2017-09-27T12:11:26.174047008Z","message":"network"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"warn","timestamp":"2017-09-27T12:11:26.174073827Z","message":"restarting"}
{"fields":{"name":"HelpHero","type":"server"},"level":"info","timestamp":"2017-09-27T12:13:05.018284557Z","message":"starting"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T12:13:05.018453866Z","message":"found free port 34201"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T12:13:05.01848986Z","message":"executing \"cd dist \u0026\u0026 node-v8.4.0-linux-x64/bin/node lib/functions\""}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T12:13:05.024101628Z","message":"proxy (pid=12) started"}
{"fields":{"app":"HelpHero","plugin":"relay","region":"us-west-2","version":"13"},"level":"info","timestamp":"2017-09-27T12:13:05.024150704Z","message":"waiting for http://<IP_ADDRESS>:34201 to listen (timeout 15s)"}

Let me know if theres anything else that would be helpful

@cloudkite interesting thanks! Looks like it is getting restarted, might have something to do with it

@cloudkite also curious why it's restarting in the first place, indicates there's some problem with the Node app

@tj Thanks will try disabling the proxy, I also realised I'm not logging uncaught exceptions so I'll see if anything shows up there.

@tj after updating to v0.2.6 and turning off proxy retrying I no longer get the Body length 0 error or any restarts in the logs however now I intermittently get
7:32:07am ERRO response duration: 0 id: a9da48dc-a60d-11e7-b0bd-69f819e1e570 method: POST path: /api/query size: 0 B stage: production status: 502 version: 17

Also nothing is being picked up by the process.on('uncaughtException' handler that I added.
I managed to replicate the 502 response once while invoking request through postman, however the exact same request succeeded the next time so I wasn't able to establish any kind of pattern.

I'm seeing up 0.2.6 return content-length zero too.
https://github.com/kaihendry/helloworld/tree/json
$ curl -sI https://hello.natalian.org/ | grep content-length
content-length: 0
x-amzn-remapped-content-length: 26

@cloudkite hmm strange I tried
const http = require('http')
const fs = require('fs')
const { PORT = 3000 } = process.env

http.createServer((req, res) => {
  setTimeout(_ => {
    asdf()
    res.setHeader('Content-Type', 'application/json')
    fs.createReadStream('up.json').pipe(res)
  }, 50)
}).listen(PORT)

process.on('uncaughtException', function(err) {
  console.log('error %s', err)
  process.exit(1)
})

and it seems ok
  2:06:30pm INFO starting name: example-app type: server
  2:06:30pm INFO found free port 34533 version: 20
  2:06:30pm INFO executing "node app.js" version: 20
  2:06:30pm INFO proxy (pid=12) started version: 20
  2:06:30pm INFO waiting for http://<IP_ADDRESS>:34533 to listen (timeout 15s) version: 20
  2:06:31pm INFO error ReferenceError: asdf is not defined
  2:06:31pm ERRO response duration: 100 id: 93af79cc-a698-11e7-ab22-5fb74f471487 ip: <IP_ADDRESS> method: GET path: / size: 0 B stage: production status: 502 version: 20

@kaihendry that's just API Gateway changing it for the HEAD request (-I), curl's -D- lets you output just the header for a GET:
$ curl -sD- `up url` | grep Content-Length
Content-Length: 12
x-amzn-Remapped-Content-Length: 12

@tj Do you think this is a logging problem or something else?
If logging is working correctly then that suggests there are no uncaught exceptions and something going wrong outside of the app in the shim or somewhere else?
Also no longer getting any mentions of restarts in the logs? Does that only happen when retrying is enabled?
Thanks for your help with this! if theres anything I can do on my end to help debug this please let me know.

@cloudkite yep, Up should be only restarting if your process crashes. I guess the only other in-app problem could be if there's a process.exit(n) in there somewhere exiting silently, but that seems unlikely.
No problem! A few people have hit this, definitely need to dig into it. I should be back to work tomorrow so I'll do some more hunting and try to improve the debug-ability since I can't seem to reproduce this stuff easily

https://groups.google.com/forum/m/#!topic/golang-codereviews/pTrfhr7L15w

Go's reverse proxy code at the bottom there mentions that a zero-length response (only appropriate for a 204 response etc) results in a 502, so I suppose it could be your app after all, some kind of state issue.
Either way it should be more obvious in Up that's for sure!

@tj I added the following log to my GraphQL API just before I stringify the body to see if I'm sending an empty or even falsy body at any point
    if (!body) {
      log.error('Empty body.', {
        id: req.headers['x-request-id'],
        status,
        headers,
        body,
      })
    }

using: up logs -s 12h 'message = "Empty body."
gives me a whole bunch of 204s so I know at least the logs for this are being entered.
using: up logs -s 12h 'message = "Empty body." and status != 204'
I get zero log entries however have still gotten a few 502s in same time period.
Unless there's something really weird going on then doesn't look like I'm ever returning an empty body.
btw the log search is really awesome 👍

@tj you mentioned that Up only restarts your process if it crashes.  That is not true (in the current code), Up will restart your process once the proxy encounters a network error.  If you want that design to be true, I would be happy to work on it (it is a better design IMO).

Ok, I have a fairly reliable pattern that I've found (and a hypothesis for why):
First, linking #313 the original issue logged and #334 since it was an initial try at fixing some of those problems.
Forensic logs redacted with:
const fs = require('fs');
const crypto = require('crypto');

const newlines = fs.readFileSync('./logs.json', 'utf8')
  .trim()
  .split('\n')
  .map(x => JSON.parse(x))
  .filter(l => l.fields.app !== true)
    .map((l) => {
    if (l.fields.path) {
      l.fields.path = crypto.createHash('sha256').update(l.fields.path, 'utf8').digest('base64').replace(/\=+$/g, '');
    }

    if (l.fields.query) {
      l.fields.query = crypto.createHash('sha256').update(l.fields.query, 'utf8').digest('base64').replace(/\=+$/g, '');
    }

    if (l.message && l.message.startsWith('executing')) {
      l.message = 'executing REDACTED';
    }

    return JSON.stringify(l);
  })
  .join('\n');

fs.writeFileSync('./redacted.txt', newlines);

redacted.txt
Patterns:
Connection reset by peer happens right before the http content length error.  And then the error happens a couple more times (presumably retries).  The connection reset always seems to happen after a significant delay from the previous request (1 minute up to 5 minutes I think).
Hypothesis:
I think we are hitting the window where the container is about to be discarded, but not yet (1-5 minutes of inactivity).  With that in mind, I think we either hitting up on bugs in the TCP stacks / http stacks of the app code, where they cannot really operate as they should after being suspended for so long.  Possibly the client app code (hapi.js for @berzniz and vanilla go for me) or amazonlinux tcp stack code is sending an RST over the connection (which after so much inactivity would probably be correct). The normal FIN-ACK sequence will not work since both sides of the TCP connection are suspended.
I think that because up-proxy is using a reverse proxy with agressive connection reuse, it tries to send a request to the application code and immediately receives an RST.  Then I believe this is exposing a bug with the retry logic (causing the http contentlength errors).
Overall, I believe the proxy has some version of this bug reading from a closed body gives nothing.

@tj @skabbes
No sure if this helps, but another thing I tried todo is logging every request inside the server callback
http.createServer((req, res) => {
  log.info('hello', { id: req.headers['x-request-id'] })
  handler(req, res)
}).listen(port)

for the requests ids that respond with 502 i never see the "hello" log, every other request gets a "hello" log entry

@cloudkite I noticed in the stdlib that the code path for this content-length/body error is used for both the request and response, they re-use the same code, so it definitely could be likely that it never reaches your app, like @skabbes mentions about the immediate reset.
I swear net/http's Transport does some retrying internally on connection resets, I remember reading that it only bails on a partial write. So maybe we're hitting that case, where part of the request body is written, and Transport thinks that it can no longer safely re-transmit.
@skabbes thanks again for digging into it! I've been sick the past week so I haven't had much time. Sounds like from @cloudkite's testing that it's the request and lines up with what you're suggesting.

need to do a more thorough read, but I think this is the spot https://github.com/golang/go/blob/ec40c7e189182918480099bd73b5c0c73018736c/src/net/http/transport.go?ts=2#L469-L472

If you have some code you want to throw into a prod environment that will repo this, let me know - I can run a custom version of up.

Maybe we should try disabling net.Dialer's keepalive, I suppose if the container is "frozen" that may not be functioning properly. Bit of a wild guess but maybe worth a shot

AHH, @tj you know what it might be??
I bet for POSTs, we are re-reading the request body twice.

Request comes in, with a body. create a faux http.request
We send it to the reverse proxy (and it consumes the request body)
We need to retry the request, but cannot read the body anymore - Error.

No matter what happens after 3, we can never re-read the body of the request, so we will never complete it.

@skabbes Does the retry workflow you described still happen if you have "proxy": { "retry": false }? as I have retries disabled and am still getting this

@cloudkite no, I haven't yet - but I can.  Do you have any logs that I can take a look at?  Feel free to use the redact script I have above.

Hmm yea seems like @cloudkite hits it without retries though that's the extra strange part haha. We could try that req.GetBody thing to recycle the body maybe, but it doesn't explain sans-retry errors

I’m also experiencing it without retries. However, it only happens for one PUT endpoint out of about 30 endpoints. Tried logging it, but I’m not sure I’m seeing the request reaching the node server

I don't have time today to debug, but I'm going to at least deploy with "retry: false" today and get logs for tomorrow.

More observations I see:
Since API Gateway has a timeout of 30 seconds, I also set the Lambda's timeout to 30 seconds. Some requests take more than 30 seconds (I have no idea why, they are not doing much). When these requests finish and respond, the connection is reset.
You can see here in the log that endpoint1 gets a 502 at the exact moment where endpoint2 responds (endpoint2 request started long before the request for endpoint1).
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Verifying Usage Plan for request: deb46ae8-aa79-11e7-9499-dfe0ba962dc3. API Key:  API Stage: b98tge5a8h/production
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) API Key  authorized because method 'ANY /{proxy+}' does not require API Key. Request will not contribute to throttle or quota limits
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Usage Plan check succeeded for API Key  and API Stage b98tge5a8h/production
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Starting execution for request: deb46ae8-aa79-11e7-9499-dfe0ba962dc3
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) HTTP Method: PUT, Resource Path: /api/endpoint1
INFO  2017-10-06T09:36:46.236Z START RequestId: deb4b96e-aa79-11e7-af84-bd8064c08cec Version: 74
INFO  2017-10-06T09:36:46.249Z 171006/093410.371, [response,api] http://ip-10-20-36-172:33499: post /api/endpoint2 {} 200 (32368ms)
INFO  2017-10-06T09:36:46.251Z 2017/10/06 09:36:46 http: proxy error: EOF
ERROR 2017-10-06T09:36:46.251Z PUT /apiendpoint1 (502) response
INFO  2017-10-06T09:36:46.252Z END RequestId: deb4b96e-aa79-11e7-af84-bd8064c08cec
INFO  2017-10-06T09:36:46.252Z REPORT RequestId: deb4b96e-aa79-11e7-af84-bd8064c08cec  Duration: 11.85 ms  Billed Duration: 100 ms   Memory Size: 768 MB Max Memory Used: 176 MB
INFO  2017-10-06T09:36:46.256Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Successfully completed execution
INFO  2017-10-06T09:36:46.256Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Method completed with status: 502

Ah, this is all starting to make sense. At least the layers of timeouts,
process suspension etc are starting to become more clear
On Fri, Oct 6, 2017, 8:46 AM Tal Bereznitskey<EMAIL_ADDRESS>wrote:

More observations I see:
Since API Gateway has a timeout of 30 seconds, I also set the Lambda's
timeout to 30 seconds. Some requests take more than 30 seconds (I have no
idea why, they are not doing much). When these requests finish and respond,
the connection is reset.
You can see here in the log that endpoint1 gets a 502 at the exact moment
where endpoint2 responds (endpoint2 request started long before the
request for endpoint1).
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Verifying Usage Plan for request: deb46ae8-aa79-11e7-9499-dfe0ba962dc3. API Key:  API Stage: b98tge5a8h/production
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) API Key  authorized because method 'ANY /{proxy+}' does not require API Key. Request will not contribute to throttle or quota limits
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Usage Plan check succeeded for API Key  and API Stage b98tge5a8h/production
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Starting execution for request: deb46ae8-aa79-11e7-9499-dfe0ba962dc3
INFO  2017-10-06T09:36:46.223Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) HTTP Method: PUT, Resource Path: /api/endpoint1
INFO  2017-10-06T09:36:46.236Z START RequestId: deb4b96e-aa79-11e7-af84-bd8064c08cec Version: 74
INFO  2017-10-06T09:36:46.249Z 171006/093410.371, [response,api] http://ip-10-20-36-172:33499: post /api/endpoint2 {} 200 (32368ms)
INFO  2017-10-06T09:36:46.251Z 2017/10/06 09:36:46 http: proxy error: EOF
ERROR 2017-10-06T09:36:46.251Z PUT /apiendpoint1 (502) response
INFO  2017-10-06T09:36:46.252Z END RequestId: deb4b96e-aa79-11e7-af84-bd8064c08cec
INFO  2017-10-06T09:36:46.252Z REPORT RequestId: deb4b96e-aa79-11e7-af84-bd8064c08cec  Duration: 11.85 ms  Billed Duration: 100 ms   Memory Size: 768 MB Max Memory Used: 176 MB
INFO  2017-10-06T09:36:46.256Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Successfully completed execution
INFO  2017-10-06T09:36:46.256Z (deb46ae8-aa79-11e7-9499-dfe0ba962dc3) Method completed with status: 502
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/apex/up/issues/276#issuecomment-334793495, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAkJMpgzvyioa9Sw7oa0M8f8Kz1Amuhyks5spktVgaJpZM4PDTdS
.

I’m looking at the _proxy.js shim file (https://github.com/apex/up/blob/master/internal/shim/index.js) and wondering: var callback seems to always remember just the last invocation’s cb function. If two requests comes one after the other, the first one could potentially responds to the cb of the second and never respond to its own cb.
Could this lead to a horrible bug or am I missing something?

@berzniz I think you're right, it would explain a lot going on here.  I think the design of that shim is that lambda will only call your handler once concurrently, but sounds like it wasn't 100% thought through.
I'm going to prototype today up examples with various timeout characteristics, and try to find a minimal repro.

I've setup this endpoint here:
https://k372i6uy51.execute-api.us-west-2.amazonaws.com/development/:timeout
Where timeout is in milliseconds, for example:
https://k372i6uy51.execute-api.us-west-2.amazonaws.com/development/5000 -> will wait 5 seconds and responds with:
{"timeout":5000,"start":"2017-10-08T20:36:07.120Z","end":"2017-10-08T20:36:12.125Z"}

I tried it out but could not reproduce. I'm keeping the example up, here's the code:
const express = require('express')
const app = express()
const { PORT = 3000 } = process.env

app.get('/:time', (req, res) => {
  let { time } = req.params
  const timeout = parseInt(time, 10) || 0
  const start = new Date()
  setTimeout(() => {
    const end = new Date()
    res.send({ timeout, start, end })
  }, timeout)
})

app.listen(PORT, () => {
  console.log(`Example app listening on port ${PORT}!`)
})

Hey Everyone, great news I have been able to reproduce contentlength 0 error with this code and a slight modification to up to allow POSTs to be retried.  The error seems to come when a request's body is re-read during a retry (Important to note, that even if your code is perfect, retries will still be necessary since suspending the lambda process suspending will really screw with your idle tcp connection on the reverse proxy).
This code fixes it: https://github.com/apex/up/pull/363
Basically go's default http transport allows for re-reading the request body with an optional function GetBody.  We simply needed to implement that.
@tj it would be great to get a release of this ASAP, since I spoke with @berzniz tonight and sounds like he's about read to ditch Up in production because of this bug.
I am running a custom build, so it is less urgent for me.

@skabbes awesome work! looking forward to giving it a test

Doing some QA and I'll have those changes in a release soon!

up upgrade for 0.2.7 with @skabbes' fixes, definitely let us know if that does that trick, note you'll have to change .lambda.timeout to .proxy.timeout (lambda stuff is implied now)

@tj I upgraded to 0.2.8 and moved the timeout to the proxy section of config and re-enabled retry. however I still get 502's :(
12:33:27pm INFO response duration: 19 id: 403798f2-ad4a-11e7-a949-abe57b013739 method: POST path: /api/query query: trace=nav-startVisit size: 1.3 kB stage: production status: 200 version: 37
  12:33:33pm WARN temporary error: read tcp <IP_ADDRESS>:51190-><IP_ADDRESS>:34961: read: connection reset by peer version: 37
  12:33:33pm ERRO network error: http: ContentLength=1131 with Body length 0 version: 37
  12:33:33pm WARN restarting version: 37
  12:33:33pm INFO found free port 40329 version: 37
  12:33:33pm INFO executing "cd dist && node-v8.6.0-linux-x64/bin/node lib/functions" version: 37
  12:33:33pm INFO proxy (pid=28) started version: 37
  12:33:33pm INFO waiting for http://<IP_ADDRESS>:40329 to listen (timeout 15s) version: 37
  12:33:34pm WARN restarted restarts: 1 version: 37
  12:33:34pm ERRO response duration: 976 id: 43c63e6d-ad4a-11e7-8bb5-7bf6da8d0b3e method: POST path: /api/query query: trace=nav-startVisit size: 0 B stage: production status: 502 version: 37
  12:33:42pm INFO response duration: 45 id: 4946eebc-ad4a-11e7-8786-4b664fc4e53d method: GET path: /embed/DLePgo8rWPR size: 55 B stage: production status: 302 version: 37

might be related to https://github.com/golang/go/issues/22158

I had a similar piece of software to up that simply created a new http.client for every request.  it isn't ideal perf-wise, but it worked reliably.  Would anyone be up for that instead?
The go issue seems to be exactly the same behavior that is being exhibited here FWIW.

My understanding is that lambda does not processes requests in parallel instead it starts other instances to handle concurrency? in which case would it even make a difference perf-wise?

I suppose very little, the overhead of establishing a new connection. The
initial TCP handshake. My vote it to try that next.
On Mon, Oct 9, 2017, 5:11 PM Jonas Budelmann<EMAIL_ADDRESS>wrote:

My understanding is that lambda does not processes requests in parallel
instead it starts other instances to handle concurrency? in which case
would it even make a difference perf-wise?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/apex/up/issues/276#issuecomment-335322604, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAkJMhIlKqCTUFy8lwYKKVv6ek-AfY8Hks5sqrYngaJpZM4PDTdS
.

Ok, I have another attempt at this:
https://github.com/apex/up/pull/365
The change is rather trivial, but the behaviors described and the resolutions match almost exactly.  I've deployed it to prod, fingers crossed.

Hmmm it does seem to have a noticeable impact on latency, seeing more timeouts now, at least with some quick non-thorough testing

Weird deployed a few more times and I think it's ok, false alarm hopefully :D. Will do some more thorough testing there to make sure we didn't compromise latency, not being broken is more important of course haha

Oh man, I'd be incredibly skeptical of lambda if their loopback network interface was that slow.
I think overall, this is likely a win for 99p latency, since it prevents the http bug (which incurs the additional latency of restarting a process), and faster responses lead to more container reuse.  And I betcha booting up a new container is a lot more latency than a TCP handshake :)

oh yea definitely, it was just odd since I was getting zero timeouts with previous versions, but seems like just that one deploy was messed for whatever reason, just Lambda acting up or something

Ah, cool. You scared me a little bit there.

Can confirm that v0.2.9 has resolved it for me 👍 👯
Thanks @tj, @skabbes!

From someone who has been watching this from the sidelines, I would like to
say i appreciate all the effort from everyone involved 👌
On Oct 11, 2017 7:25 AM, "Jonas Budelmann"<EMAIL_ADDRESS>wrote:
Can confirm that v0.2.9 has resolved it for me 👍 👯
Thanks @tj https://github.com/tj, @skabbes https://github.com/skabbes!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/apex/up/issues/276#issuecomment-335678550, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AE7LUTei9lybunn5eLojAM8PpIORDUwJks5srEMpgaJpZM4PDTdS
.

Big thanks to everyone from me too, and early adopters of course! Super helpful to have all the feedback

Sounds like we're good to close, thanks everyone!


THCI: convert PSKc string to hex format.

Test case 9.2.9 checks whether or not pending dataset contains PSKc TLV. Hence convert it from string format to hex format first, that would help send dataset pskc command to reference unit successfully.

Current coverage is 73.58% (diff: 100%)

Merging #916 into master will decrease coverage by 0.05%

@@             master       #916   diff @@
==========================================
  Files           116        116
  Lines         17273      17273
  Methods        2412       2410     -2
  Messages          0          0
  Branches       2094       2094
==========================================
- Hits          12721      12711    -10
- Misses         3848       3853     +5
- Partials        704        709     +5

Powered by Codecov. Last update dc6ad5b...a065c58