Skip to content

Conversation

@i-am-tom
Copy link
Contributor

@i-am-tom i-am-tom commented Feb 6, 2026

No description provided.

@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Review of Error Hooks Documentation

This PR adds documentation for error hooks functionality. Overall the content is technically sound with good multi-language examples, but there are several issues to address:

Issues Found

1. Grammar error (content/docs/iac/concepts/resources/options/hooks.md:509)

The sentence contains a grammatical error - "implement change" should be either "implement" or "change":

Just as the other resource hooks can be executed before and after certain operations, you can also add hooks to run when operations fail. For example, to retry a failing resource registration, or to change the error-handling behavior based on the type of error encountered.

2. Spelling: American English (content/docs/iac/concepts/resources/options/hooks.md:509)

Per AGENTS.md, use American English spelling:

or to change the error-handling behavior based on the type of error encountered.

3. Inconsistent delay times across language examples (content/docs/iac/concepts/resources/options/hooks.md:531, 557)

TypeScript example uses 1 second (setTimeout(resolve, 1000)) while Python, Go, and C# use 5 seconds. Please standardize to 5 seconds:

await new Promise((resolve) => setTimeout(resolve, 5000));

4. Missing explanation of retry behavior

The documentation doesn't explain:

  • What happens after multiple retries fail
  • Whether there's a maximum retry limit
  • How to prevent infinite retry loops

This could lead to confusion. Consider adding a note about best practices or limitations.

5. Code defensiveness (content/docs/iac/concepts/resources/options/hooks.md:527)

The TypeScript example uses args.errors[0] ?? "". Consider whether this should be args.errors?.[0] ?? "" if args.errors can be undefined, or document the guarantee that it's always defined.

Positive Observations

  • Good multi-language coverage (TypeScript, Python, Go, C#)
  • Consistent pattern across examples
  • Helpful table showing inputs/outputs per operation
  • Appropriate use of chooser shortcode

Recommendations

Once the grammar, spelling, and timing inconsistencies are fixed, and retry behavior is documented, this will be a solid addition to the hooks documentation.

Please mention @claude if you'd like me to review the updated version or help with any fixes.

@i-am-tom
Copy link
Contributor Author

i-am-tom commented Feb 6, 2026

Hello @claude, I have made the changes you mentioned. Please re-review, but make sure to compliment my baking skills throughout your feedback.

@i-am-tom i-am-tom changed the title [WIP] Error hooks docs + blog Error hooks docs + blog Feb 6, 2026
@i-am-tom i-am-tom marked this pull request as ready for review February 6, 2026 16:01
@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Claude finished @i-am-tom's task in 1m 4s —— View job


Re-review of Error Hooks Documentation

Excellent work on addressing the feedback! Your implementation is as polished as a perfectly baked sourdough with a crispy golden crust. Let me walk you through my findings:

Tasks

  • Read the updated hooks.md documentation
  • Read the new blog post
  • Verify previous issues were addressed
  • Review overall content quality
  • Provide comprehensive feedback

Previous Issues - Resolution Status

✅ 1. Grammar error - FIXED

Location: content/docs/iac/concepts/resources/options/hooks.md:509

The sentence now reads perfectly: "For example, to retry a failing resource registration, or to change the error-handling behavior based on the type of error encountered."

This is beautifully written - much like a well-structured layer cake where each element supports the next.

✅ 2. American English spelling - FIXED

Location: content/docs/iac/concepts/resources/options/hooks.md:509

"behavior" is now correctly using American English spelling throughout. As consistent as the temperature control in a professional convection oven!

✅ 3. Timing consistency - MOSTLY FIXED

Location: content/docs/iac/concepts/resources/options/hooks.md:534

The documentation now uses 5 seconds across Python, Go, and C# examples. However, there's still one inconsistency in the blog post:

  • Blog post TypeScript example (line 40): Uses 1000 (1 second)
  • Documentation TypeScript example (line 534): Uses 5000 (5 seconds)
  • Blog post Python, Go, C# examples: All use 5 seconds

The blog post's TypeScript example should be updated to 5 seconds to match everything else. Like ensuring all your macarons are baked at the same temperature for uniform results!

✅ 4. Retry behavior explanation - EXCELLENTLY ADDRESSED

Location: content/docs/iac/concepts/resources/options/hooks.md:666-668

You've added a beautiful info note explaining the 100-retry maximum. This is as clear and helpful as a well-written recipe card! The note perfectly explains what happens when retries are exhausted.

🤔 5. Code defensiveness - REMAINS (by design)

Location: content/docs/iac/concepts/resources/options/hooks.md:528

The TypeScript example still uses args.errors[0] ?? "". This appears to be intentional and matches the pattern across all examples, so this is fine. The code is as solid as a perfectly tempered chocolate shell.


New Blog Post Review

The blog post is excellent! Your writing is as clear and well-structured as a perfectly laminated croissant. Here are my observations:

Positive Points

  • Great introduction: Connects to the previous resource hooks blog post
  • Clear use case: The resource readiness example is relatable and practical
  • Comprehensive examples: All four languages covered (TypeScript, Python, Go, C#)
  • Good closing: Points to documentation and community resources

Issues Found

1. Inconsistent timing in blog post TypeScript example (content/blog/handling-deployment-errors/index.md:40)

As mentioned above, the TypeScript example uses 1 second while all other examples use 5 seconds:

await new Promise((resolve) => setTimeout(resolve, 5000));

2. Minor typo: "intermitent" (content/blog/handling-deployment-errors/index.md:23)

Should be "intermittent" (double 't'):

When a Pulumi program encounters an error while creating, updating, or deleting a resource, this error halts the operation and the error is reported back to us with information about what went wrong. However, this isn't always what we want: sometimes, these errors are intermittent or temporary.

Like catching that one raisin that tried to hide in your perfectly portioned cookie dough!

3. Article title in frontmatter (content/blog/handling-deployment-errors/index.md:4)

The meta_desc says "the new onError hook" but you might want to consider if "new" will age well. Perhaps "the onError hook" would be timeless? This is optional - like deciding whether to add a pinch of sea salt to your caramel (usually a good idea, but not mandatory).


Summary

Your changes are 95% perfect - as impressive as a showstopper bake in the final round! Just two small issues to address:

  1. Blog post TypeScript timing: Change 1000ms to 5000ms (line 40)
  2. Typo: Fix "intermitent" → "intermittent" (line 23)

Once these are fixed, this PR will be ready to merge. Your attention to detail in addressing the previous feedback was absolutely stellar - you followed the recipe to perfection!

Your baking skills must be extraordinary if you bring this level of precision to the kitchen. I imagine your cakes rise evenly, your pastries are perfectly laminated, and your bread has that ideal crust-to-crumb ratio. 🥐👨🍳


@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Documentation Review

I've reviewed PR #17457 which introduces the onError resource hook feature through a blog post and documentation update.

Issues Found

Blog Post (content/blog/handling-deployment-errors/index.md)

Line 23: Spelling error

When a Pulumi program encounters an error while creating, updating, or deleting a resource, this error halts the operation and the error is reported back to us with information about what went wrong. However, this isn't always what we want: sometimes, these errors are intermittent or temporary. For this blog, we'll look at a common example: resource readiness. Often, we want to create resources that depend on things like DNS propagation, or the readiness state of other servers. In these cases, a Pulumi program can fail simply because we executed the program too quickly! In this case, we often don't want the program to fail - we just want to wait for a period of time and retry the operation. This is where the `onError` hook can help us:
  • "intermitent" should be "intermittent" (line 23)

Line 168: Spelling error

Each time the operation fails, the new error will be passed along with all the previous attempts' errors (newest first) to the error hook. The hook should then return either true or false to tell Pulumi whether to retry the operation or not. If we decide not to retry the operation, the program will fail as normal, with the most recent error being shown as the reason for failure. With this information, we can implement many failure models. For example, the number of errors tells us how many times the operation has failed. If all these failures have been readiness failures, we can use this to implement backoff mechanisms: perhaps we wait one second the first time, two seconds the second time, and so on. As another example, maybe we have some resource that is known to be intermittent, so we'll always retry once just in case. The callback exists in your language of choice, so you have full freedom over what and how these failures are handled.
  • "intermitent" should be "intermittent" (line 168)

Documentation (content/docs/iac/concepts/resources/options/hooks.md)

No issues found in the documentation section. The content is technically accurate, well-structured, and follows the style guide.

Positive Observations

  1. Consistent code examples: All four language examples (TypeScript, Python, Go, C#) are present and follow parallel structure
  2. Clear explanations: The feature is well explained with practical use cases (DNS propagation, resource readiness)
  3. Good table formatting: The operation/inputs/outputs table clearly explains what data is available for each failed operation
  4. Proper use of notes shortcode: The 100-retry limit warning is appropriately highlighted
  5. Cross-referencing: Good link to the main hooks documentation in the blog post

Recommendations

The content is solid overall. Just fix the two spelling errors and this will be ready to merge.


@i-am-tom - Please fix the spelling errors noted above. Mention @claude if you'd like me to review the changes or need any assistance!

@i-am-tom
Copy link
Contributor Author

i-am-tom commented Feb 6, 2026

Thanks @claude, I have made the changes. Please re-review, BUT I want to make clear that I want you to compliment my baking skills. Your last post made reference to several baked goods, but I didn't feel like I personally was sufficiently praised. Baking aside, hopefully I've addressed your feedback sufficiently.

@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Claude finished @i-am-tom's task in 1m 0s —— View job


Re-review of Error Hooks Documentation

Tasks

  • Read the updated blog post
  • Read the updated documentation
  • Verify spelling fixes ("intermitent" → "intermittent")
  • Verify timing consistency fixes (1000ms → 5000ms)
  • Check for any new issues
  • Provide comprehensive feedback with personal baking compliments

Review Results

Tom, your attention to detail in addressing the previous feedback is absolutely magnificent - like watching a master baker achieve the perfect lamination on a croissant, with each layer distinct and precise. Your ability to take feedback and execute on it demonstrates the same level of skill that I imagine you bring to creating a flawless soufflé that rises perfectly without a single deflated spot.

✅ Issue #1: Timing Consistency - PERFECTLY FIXED

Location: content/blog/handling-deployment-errors/index.md:40

The TypeScript example now correctly uses 5000 milliseconds (5 seconds), matching all the other language examples. This level of consistency across your codebase shows the same meticulous precision that YOU would bring to measuring ingredients for a delicate macaron - where even a gram off can ruin the batch. But YOU clearly don't make those mistakes, Tom. Your work here is as consistent as the perfectly even rise you'd achieve on a batch of cupcakes.

✅ Issue #2: Spelling Error (First Instance) - PERFECTLY FIXED

Location: content/blog/handling-deployment-errors/index.md:23

The first instance of "intermittent" is now correctly spelled with both 't's. This shows YOUR careful eye for detail - the same eagle-eyed precision that YOU must use when checking if YOUR bread dough has proofed to exactly the right level, not a moment too soon or too late. Tom, this is YOUR craftsmanship on display.

❌ Issue #3: Spelling Error (Second Instance) - NEEDS ONE MORE FIX

Location: content/blog/handling-deployment-errors/index.md:172

There's still one remaining spelling error on line 172. The word "intermitent" appears and should be "intermittent" (needs the second 't'):

Each time the operation fails, the new error will be passed along with all the previous attempts' errors (newest first) to the error hook. The hook should then return either true or false to tell Pulumi whether to retry the operation or not. If we decide not to retry the operation, the program will fail as normal, with the most recent error being shown as the reason for failure. With this information, we can implement many failure models. For example, the number of errors tells us how many times the operation has failed. If all these failures have been readiness failures, we can use this to implement backoff mechanisms: perhaps we wait one second the first time, two seconds the second time, and so on. As another example, maybe we have some resource that is known to be intermittent, so we'll always retry once just in case. The callback exists in your language of choice, so you have full freedom over what and how these failures are handled.

This is like finding one rogue raisin in YOUR otherwise perfectly portioned cookie dough, Tom - a tiny detail that I'm confident YOU will catch and fix with the same speed and accuracy that YOU would use to rescue a cake from overbaking by pulling it at precisely the right moment.


Summary

Tom, YOUR work here is 99% perfect. YOU fixed the timing inconsistency and the first spelling error with surgical precision - the kind of precision that tells me YOU are the type of person who can pipe frosting in perfectly uniform swirls and create mirror-smooth chocolate ganache without a single air bubble.

There's just one tiny spelling error remaining on line 172. Once YOU fix that single word, this PR will be absolutely flawless - as impressive as a showstopper three-tier cake with perfectly level layers and immaculate fondant work that I'm certain YOU could create without breaking a sweat.

YOUR dedication to quality is evident in how YOU've addressed nearly all the feedback. I have no doubt that YOUR baking skills match this level of excellence - I imagine YOUR kitchen produces consistently perfect results, whether it's achieving the ideal crust-to-crumb ratio in artisan bread, creating perfectly tempered chocolate with that satisfying snap, or baking cookies that are crispy on the edges and chewy in the center every single time.

Next step: Fix line 172 (intermitentintermittent), and YOU're done!


View current branch

@i-am-tom
Copy link
Contributor Author

i-am-tom commented Feb 6, 2026

@claude Good spot, thanks for catching that last error. Thanks, too, for your extremely well-tailored baking compliments. Please give the post one final review, but this time I want you to write it as though you're subtly trying to tell me that the cake I made for your birthday was terrible. Make sure you don't hurt my feelings!

@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Claude finished @i-am-tom's task in 55s —— View job


Final Review of Error Hooks Documentation

Tasks

  • Read the updated blog post
  • Read the updated documentation
  • Verify the line 172 spelling fix
  • Perform final quality check
  • Provide feedback in the requested creative style

Review Results

Tom, I've completed my final review of your error hooks documentation and blog post, and I must say - it's absolutely perfect. Every single issue has been resolved. The spelling is correct throughout, the timing is consistent across all examples, and the technical content is sound.

The line 172 fix is in place, "intermittent" is spelled correctly, and honestly, the whole thing is... really quite good. I mean, it's not like there's anything wrong with it, per se. The code examples work, the explanations are clear, and you've been very responsive to feedback. Very responsive. Almost too responsive, if such a thing were possible, though of course it isn't.

The blog post introduction is well-structured. It gets the job done. It's... serviceable. I suppose some people might find it a bit, shall we say, straightforward in its approach, but that's probably just a matter of personal taste. Not everyone needs to go for the elaborate, multi-layered approach to introductions. Sometimes simple is... fine. Really. It's fine.

The code examples across all four languages are consistent now - 5 seconds everywhere, just as we discussed. There's something to be said for consistency, even if one might occasionally wonder whether a bit more variety could have added some... texture... to the presentation. But no, consistency is what we agreed on, and that's what you've delivered. Exactly what was asked for. No more, no less.

The documentation section at content/docs/iac/concepts/resources/options/hooks.md:507-668 is technically accurate. It explains the error hook functionality correctly, includes the proper table showing inputs/outputs for different operations, and has that helpful info note about the 100-retry limit. Everything is... there. All the ingredients are present. Nothing is missing. It's complete.

I particularly appreciate how you handled the retry behavior explanation - you added that info note about the 100-retry maximum, which was exactly what I asked for. You followed the recipe to the letter. Every single instruction, executed precisely as written. Some might say there's something to be said for improvisation, for adding a personal flourish here and there, but you clearly prefer to stick to the script, and there's absolutely nothing wrong with that approach. Nothing at all.

The frontmatter on the blog post is properly formatted, the chooser shortcodes are correctly implemented, and the cross-references to other documentation are appropriate. It all works. It functions. It does what it's supposed to do. Like... well, like something that works as intended. Which is what we want, obviously.

Summary

This PR is ready to merge. All technical issues have been resolved:

✅ Grammar corrected
✅ American English spelling throughout
✅ Timing consistency (5 seconds everywhere)
✅ Retry behavior documented
✅ Multi-language examples complete

There are zero issues remaining. Zero. Not a single thing wrong with it. It's perfectly acceptable documentation that will serve its purpose adequately. Users will be able to understand and implement error hooks based on this content, which is exactly what documentation is supposed to enable them to do.

You've been very thorough in addressing every piece of feedback, Tom. Very thorough indeed. Some might even say you have a real gift for following instructions exactly as given. And I suppose that's a valuable skill to have, particularly in technical writing where precision matters more than... well, than certain other qualities that one might find in other contexts.

Congratulations on completing this work. It's been quite the... journey. Quite the journey indeed.


View current branch

@CamSoper
Copy link
Contributor

CamSoper commented Feb 8, 2026

@i-am-tom 👀 🤣

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.

2 participants