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: TypeError with hasOwnProperty in deepFindPathToProperty #234

Merged
merged 3 commits into from
Oct 11, 2024

Conversation

kachick
Copy link
Contributor

@kachick kachick commented Sep 24, 2024

Resolves #58 with fix to handle non object values
Resolves #59 (comment) with adding unit test

Background is written in #137 (comment)

This change still returns errors for caller, but replaced to correct errors such as MissingPageInfo instead of TypeError.
@stergion @nickfloyd How do you think this direction?


Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No - As far as I know, I don't expect braking change in this PR, if you find, please tell me

Copy link

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@wolfy1339 wolfy1339 added the Type: Bug Something isn't working as documented label Sep 25, 2024
@wolfy1339 wolfy1339 merged commit 7b17cc4 into octokit:main Oct 11, 2024
9 checks passed
@kachick
Copy link
Contributor Author

kachick commented Oct 11, 2024

Thank you for merging 🙏


Looks like release CI was failed from closed timing of #59

log
 + @octokit/[email protected]
[3:04:25 PM] [semantic-release] [@semantic-release/npm] › ℹ  Published @octokit/[email protected] to dist-tag @latest on https://registry.npmjs.org/
[3:04:25 PM] [semantic-release] › ✔  Completed step "publish" of plugin "@semantic-release/npm"
[3:04:25 PM] [semantic-release] › ℹ  Start step "success" of plugin "@semantic-release/github"
[3:04:27 PM] [semantic-release] › ✘  Failed step "success" of plugin "@semantic-release/github"
[3:04:27 PM] [semantic-release] › ✘  An error occurred while running semantic-release: Error: Could not resolve to an Issue with the number of 59.
    at file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/aggregate-error/index.js:23:26
    at Array.map (<anonymous>)
    at new AggregateError (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/aggregate-error/index.js:16:19)
    at file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/lib/plugins/pipeline.js:55:13
    at async pluginsConfigAccumulator.<computed> [as success] (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/lib/plugins/index.js:87:11)
    at async run (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/index.js:218:3)
    at async Module.default (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/index.js:278:22)
    at async default (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/cli.js:55:5) {
  type: 'NOT_FOUND',
  path: [ 'repository', 'issue59' ],
  locations: [ { line: 54, column: 14 } ],
  pluginName: '@semantic-release/github'
}
AggregateError: 
    Error: Could not resolve to an Issue with the number of 59.
        at Array.map (<anonymous>)
        at file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/lib/plugins/pipeline.js:55:13
        at async pluginsConfigAccumulator.<computed> [as success] (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/lib/plugins/index.js:87:11)
        at async run (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/index.js:218:3)
        at async Module.default (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/index.js:278:22)
        at async default (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/cli.js:55:5)
    at file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/lib/plugins/pipeline.js:55:13
    at async pluginsConfigAccumulator.<computed> [as success] (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/lib/plugins/index.js:87:11)
    at async run (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/index.js:218:3)
    at async Module.default (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/index.js:278:22)
    at async default (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/cli.js:55:5) {
  errors: [
    Error: Could not resolve to an Issue with the number of 59.
        at file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/aggregate-error/index.js:23:26
        at Array.map (<anonymous>)
        at new AggregateError (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/aggregate-error/index.js:16:19)
        at file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/lib/plugins/pipeline.js:55:13
        at async pluginsConfigAccumulator.<computed> [as success] (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/lib/plugins/index.js:87:11)
        at async run (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/index.js:218:3)
        at async Module.default (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/index.js:278:22)
        at async default (file:///home/runner/.npm/_npx/cdf31b77322f1d44/node_modules/semantic-release/cli.js:55:5) {
      type: 'NOT_FOUND',
      path: [Array],
      locations: [Array],
      pluginName: '@semantic-release/github'
    }
  ]
}

However, https://github.com/octokit/plugin-paginate-graphql.js/releases/tag/v5.2.4 has been released.

Please ping me if I need to take any action regarding that.

@wolfy1339
Copy link
Member

No action is necessary. As long as the package was released on NPM and GitHub, it's all good.

I'm not quite sure why it failed. It only failed at posting the comments on closed issues/PRs

@kachick
Copy link
Contributor Author

kachick commented Oct 11, 2024

Thanks for sharing it 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG]: deepFindPathToProperty() throws TypeError if graphql response field is null
2 participants